From d49b885269d7ec5661fd97a3fde564f46cd30e43 Mon Sep 17 00:00:00 2001 From: Michal Skrivanek Date: Thu, 25 Sep 2025 19:50:20 +0200 Subject: [PATCH 1/2] add checking of running container on each exporter host relies on labels jumpstarter.version and jumpstarter.revision on image, or standard OCI labels org.opencontainers.image.version and org.opencontainers.image.revision --- cmd/apply.go | 4 + internal/config/config.go | 15 ++-- internal/config/loader.go | 37 +++++++++ internal/container/version.go | 96 +++++++++++++++++++++ internal/exporter/ssh/ssh.go | 152 +++++++++++++++++++++++++++------- 5 files changed, 271 insertions(+), 33 deletions(-) create mode 100644 internal/container/version.go diff --git a/cmd/apply.go b/cmd/apply.go index bf69e8b..66b5883 100644 --- a/cmd/apply.go +++ b/cmd/apply.go @@ -60,6 +60,10 @@ var applyCmd = &cobra.Command{ } config_lint.Validate(cfg) + if err := config_lint.ValidateWithError(cfg); err != nil { + return fmt.Errorf("config validation failed: %w", err) + } + tapplier, err := templating.NewTemplateApplier(cfg, nil) if err != nil { return fmt.Errorf("error creating template applier %w", err) diff --git a/internal/config/config.go b/internal/config/config.go index f047110..08dedbe 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -4,15 +4,17 @@ import ( "os" "path/filepath" + "github.com/jumpstarter-dev/jumpstarter-lab-config/internal/container" "gopkg.in/yaml.v3" ) // Config represents the structure of the jumpstarter-lab.yaml file. type Config struct { - Sources Sources `yaml:"sources"` - Variables []string `yaml:"variables"` - BaseDir string `yaml:"-"` // Not serialized, set programmatically - Loaded *LoadedLabConfig `yaml:"-"` // Not serialized, used internally + Sources Sources `yaml:"sources"` + Variables []string `yaml:"variables"` + BaseDir string `yaml:"-"` // Not serialized, set programmatically + Loaded *LoadedLabConfig `yaml:"-"` // Not serialized, used internally + ContainerVersions map[string]*container.ImageLabels `yaml:"-"` // Not serialized, container versions by image URL } // Sources defines the paths for various configuration files. @@ -43,6 +45,9 @@ func LoadConfig(filePath string, vaultPassFile string) (*Config, error) { cfg.BaseDir = filepath.Dir(filePath) cfg.Loaded, err = LoadAllResources(&cfg, vaultPassFile) + if err != nil { + return nil, err + } - return &cfg, err + return &cfg, nil } diff --git a/internal/config/loader.go b/internal/config/loader.go index 4887ed0..b00c9c8 100644 --- a/internal/config/loader.go +++ b/internal/config/loader.go @@ -9,6 +9,7 @@ import ( jsApi "github.com/jumpstarter-dev/jumpstarter-controller/api/v1alpha1" api "github.com/jumpstarter-dev/jumpstarter-lab-config/api/v1alpha1" + "github.com/jumpstarter-dev/jumpstarter-lab-config/internal/container" "github.com/jumpstarter-dev/jumpstarter-lab-config/internal/vars" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -305,9 +306,45 @@ func LoadAllResources(cfg *Config, vaultPassFile string) (*LoadedLabConfig, erro } } + // Retrieve container versions for all unique container images found in exporters + containerVersions := retrieveContainerVersionsFromExporters(loaded) + + // Store the container versions in the config + cfg.ContainerVersions = containerVersions + return loaded, nil } +// retrieveContainerVersionsFromExporters retrieves container versions for all unique container images found in exporters +func retrieveContainerVersionsFromExporters(loaded *LoadedLabConfig) map[string]*container.ImageLabels { + containerVersions := make(map[string]*container.ImageLabels) + uniqueImages := make(map[string]bool) + + // Collect all unique container images from exporter config templates + for _, template := range loaded.ExporterConfigTemplates { + if template.Spec.ContainerImage != "" { + uniqueImages[template.Spec.ContainerImage] = true + } + } + + // Retrieve version information for each unique image + for imageURL := range uniqueImages { + imageLabels, err := container.GetImageLabelsFromRegistry(imageURL) + if err != nil { + fmt.Printf("Latest container version of %s: unavailable (%v)\n", imageURL, err) + containerVersions[imageURL] = &container.ImageLabels{} // Store empty labels + } else if imageLabels.IsEmpty() { + fmt.Printf("Latest container version of %s: no version info available\n", imageURL) + containerVersions[imageURL] = imageLabels + } else { + fmt.Printf("Latest container version of %s: %s %s\n", imageURL, imageLabels.Version, imageLabels.Revision) + containerVersions[imageURL] = imageLabels + } + } + + return containerVersions +} + func ReportLoading(cfg *Config) { fmt.Println("Reading files from:") diff --git a/internal/container/version.go b/internal/container/version.go new file mode 100644 index 0000000..5682114 --- /dev/null +++ b/internal/container/version.go @@ -0,0 +1,96 @@ +package container + +import ( + "encoding/json" + "fmt" + "os/exec" + "strings" +) + +// ImageLabels represents the labels from a container image +type ImageLabels struct { + Version string + Revision string +} + +// GetImageLabelsFromRegistry retrieves image labels from a registry using skopeo +func GetImageLabelsFromRegistry(imageURL string) (*ImageLabels, error) { + // Always add the docker:// prefix for skopeo + imageURL = "docker://" + imageURL + + cmd := exec.Command("skopeo", "inspect", "--override-os", "linux", "--override-arch", "amd64", imageURL) + output, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("failed to inspect image %s with skopeo: %w", imageURL, err) + } + + var imageInfo struct { + Labels map[string]string `json:"Labels"` + } + + if err := json.Unmarshal(output, &imageInfo); err != nil { + return nil, fmt.Errorf("failed to parse skopeo output: %w", err) + } + + // Get both label sets + jumpstarterVersion := imageInfo.Labels["jumpstarter.version"] + jumpstarterRevision := imageInfo.Labels["jumpstarter.revision"] + ociVersion := imageInfo.Labels["org.opencontainers.image.version"] + ociRevision := imageInfo.Labels["org.opencontainers.image.revision"] + + // Use jumpstarter labels if both exist, otherwise fall back to OCI labels + var version, revision string + if jumpstarterVersion != "" && jumpstarterRevision != "" { + version = jumpstarterVersion + revision = jumpstarterRevision + } else { + version = ociVersion + revision = ociRevision + } + + // Only return empty labels if BOTH version and revision are completely missing + // (neither jumpstarter nor OCI labels exist) + return &ImageLabels{ + Version: version, + Revision: revision, + }, nil +} + +// GetRunningContainerLabels retrieves labels from a running container using podman inspect +func GetRunningContainerLabels(serviceName string) (*ImageLabels, error) { + cmd := exec.Command("podman", "inspect", "--format", + "{{index .Config.Labels \"jumpstarter.version\"}} {{index .Config.Labels \"jumpstarter.revision\"}}", + serviceName) + output, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("failed to inspect container %s: %w", serviceName, err) + } + + parts := strings.Fields(strings.TrimSpace(string(output))) + if len(parts) < 2 { + return &ImageLabels{}, nil // Return empty labels if not found + } + + return &ImageLabels{ + Version: parts[0], + Revision: parts[1], + }, nil +} + +// CompareVersions compares two ImageLabels and returns true if they match +func (il *ImageLabels) Matches(other *ImageLabels) bool { + return il.Version == other.Version && il.Revision == other.Revision +} + +// IsEmpty returns true if both version and revision are empty +func (il *ImageLabels) IsEmpty() bool { + return il.Version == "" && il.Revision == "" +} + +// String returns a string representation of the image labels +func (il *ImageLabels) String() string { + if il.IsEmpty() { + return "no version info" + } + return fmt.Sprintf("version=%s revision=%s", il.Version, il.Revision) +} diff --git a/internal/exporter/ssh/ssh.go b/internal/exporter/ssh/ssh.go index 8bd8a6f..08f599d 100644 --- a/internal/exporter/ssh/ssh.go +++ b/internal/exporter/ssh/ssh.go @@ -14,6 +14,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/jumpstarter-dev/jumpstarter-lab-config/api/v1alpha1" + "github.com/jumpstarter-dev/jumpstarter-lab-config/internal/container" "github.com/pkg/sftp" "golang.org/x/crypto/ssh" "golang.org/x/crypto/ssh/agent" @@ -29,6 +30,10 @@ const ( BOOTC_NOT_MANAGED ) +const ( + noValuePlaceholder = "" +) + type HostManager interface { Status() (string, error) NeedsUpdate() (bool, error) @@ -186,12 +191,12 @@ func (m *SSHHostManager) Apply(exporterConfig *v1alpha1.ExporterConfigTemplate, if !dryRun { _, enableErr := m.runCommand("systemctl restart " + fmt.Sprintf("%q", serviceName)) if enableErr != nil { - fmt.Printf(" ❌ Failed to start service %s: %v\n", serviceName, enableErr) + fmt.Printf(" ❌ Failed to start service %s: %v\n", serviceName, enableErr) } else { - fmt.Printf(" ✅ Service %s started\n", serviceName) + fmt.Printf(" ✅ Service %s started\n", serviceName) } } else { - fmt.Printf(" 📄 Would restart service %s\n", serviceName) + fmt.Printf(" 📄 Would restart service %s\n", serviceName) } } @@ -217,9 +222,9 @@ func (m *SSHHostManager) Apply(exporterConfig *v1alpha1.ExporterConfigTemplate, if m.GetBootcStatus() == BOOTC_UPDATING { if dryRun { - fmt.Printf(" 📄 Bootc upgrade in progress, would skip exporter service restarts/container updates\n") + fmt.Printf(" 📄 Bootc upgrade in progress, would skip exporter service restarts/container updates\n") } else { - fmt.Printf(" ⚠ī¸ Bootc upgrade in progress, skipping exporter service restarts/container updates\n") + fmt.Printf(" ⚠ī¸ Bootc upgrade in progress, skipping exporter service restarts/container updates\n") return nil } } @@ -245,39 +250,130 @@ func (m *SSHHostManager) Apply(exporterConfig *v1alpha1.ExporterConfigTemplate, } else { // Check if service is running and start if needed statusResult, err := m.runCommand("systemctl is-active " + fmt.Sprintf("%q", svcName)) - if err != nil || statusResult.Stdout != "active\n" { - fmt.Printf(" ⚠ī¸ Service %s is not running...\n", svcName) + serviceRunning := err == nil && statusResult.Stdout == "active\n" + + if !serviceRunning { + fmt.Printf(" ⚠ī¸ Service %s is not running...\n", svcName) restartService(svcName, dryRun) + } else { + // Only check container version if service is running + err = m.checkContainerVersion(exporterConfig, svcName, dryRun, restartService) + if err != nil { + return fmt.Errorf("container version check failed: %w", err) + } } + } - // Check if container needs updating using podman auto-update (if podman exists) - autoUpdateResult, err := m.runCommand(fmt.Sprintf("command -v podman >/dev/null 2>&1 && podman auto-update --dry-run --format json | jq -r '.[] | select(.ContainerName == %q) | .Updated'", svcName)) - if err != nil { - fmt.Printf(" ℹī¸ Podman not available, skipping auto-update check\n") + return nil +} + +// checkContainerVersion checks if container needs updating using detailed version comparison +func (m *SSHHostManager) checkContainerVersion(exporterConfig *v1alpha1.ExporterConfigTemplate, svcName string, dryRun bool, restartService func(string, bool)) error { + // Only check version for container-based exporters + if exporterConfig.Spec.SystemdContainerTemplate == "" { + return nil + } + + // Check detailed version comparison (if we have container image info) + if exporterConfig.Spec.ContainerImage != "" { + return m.checkDetailedContainerVersion(exporterConfig.Spec.ContainerImage, svcName, dryRun, restartService) + } + + // No container image specified, nothing to check + return nil +} + +// checkDetailedContainerVersion performs detailed version comparison using skopeo and podman inspect +func (m *SSHHostManager) checkDetailedContainerVersion(containerImage, svcName string, dryRun bool, restartService func(string, bool)) error { + // Get expected version from registry + expectedLabels, err := container.GetImageLabelsFromRegistry(containerImage) + if err != nil { + fmt.Printf(" ⚠ī¸ Could not check container version: %v\n", err) + return nil // Don't fail the entire operation, just skip version check + } + + if expectedLabels.IsEmpty() { + fmt.Printf(" ℹī¸ No version info available for image %s\n", containerImage) + return nil // Don't fail, just skip version check + } + + // Get running container version + runningLabels, err := m.getRunningContainerLabels(svcName) + if err != nil { + fmt.Printf(" ⚠ī¸ Could not check running container version: %v\n", err) + return nil // Container might not be running yet, which is fine + } + + // Compare versions + if expectedLabels.Matches(runningLabels) { + if dryRun { + fmt.Printf(" ✅ Exporter container image running latest version\n") + } + // In non-dry-run mode, print nothing for matching versions as requested + } else { + if dryRun { + fmt.Printf(" 🔄 Would restart service for container update (running: %s, latest: %s)\n", + runningLabels.String(), expectedLabels.String()) } else { - updatedOutput := strings.TrimSpace(autoUpdateResult.Stdout) - switch updatedOutput { - case "": - fmt.Printf(" ⚠ī¸ Container %s not found in auto-update check\n", svcName) - case "false": - if dryRun { - fmt.Printf(" ✅ Exporter container image is up to date\n") - } - case "pending": - if dryRun { - fmt.Printf(" 📄 Would update container %s\n", svcName) - } else { - restartService(svcName, dryRun) - } - default: - return fmt.Errorf("unexpected auto-update result: %s", updatedOutput) - } + fmt.Printf(" 🔄 Restarting service for container update (running: %s, latest: %s)\n", + runningLabels.String(), expectedLabels.String()) + restartService(svcName, dryRun) } } return nil } +// getRunningContainerLabels gets container labels from running container +func (m *SSHHostManager) getRunningContainerLabels(serviceName string) (*container.ImageLabels, error) { + // Try jumpstarter labels first, then fall back to OCI standard labels + result, err := m.runCommand(fmt.Sprintf("podman inspect --format '{{index .Config.Labels \"jumpstarter.version\"}}\n{{index .Config.Labels \"jumpstarter.revision\"}}\n{{index .Config.Labels \"org.opencontainers.image.version\"}}\n{{index .Config.Labels \"org.opencontainers.image.revision\"}}' %s", serviceName)) + if err != nil { + return nil, fmt.Errorf("failed to inspect container %s: %w", serviceName, err) + } + + parts := strings.Split(strings.TrimSpace(result.Stdout), "\n") + // Pad with empty strings if we got fewer parts + for len(parts) < 4 { + parts = append(parts, "") + } + + jumpstarterVersion := parts[0] // jumpstarter.version + jumpstarterRevision := parts[1] // jumpstarter.revision + ociVersion := parts[2] // org.opencontainers.image.version + ociRevision := parts[3] // org.opencontainers.image.revision + + // Clean up "" to empty string + if jumpstarterVersion == noValuePlaceholder { + jumpstarterVersion = "" + } + if jumpstarterRevision == noValuePlaceholder { + jumpstarterRevision = "" + } + if ociVersion == noValuePlaceholder { + ociVersion = "" + } + if ociRevision == noValuePlaceholder { + ociRevision = "" + } + + // Use jumpstarter labels if both exist, otherwise fall back to OCI labels + var version, revision string + if jumpstarterVersion != "" && jumpstarterRevision != "" { + version = jumpstarterVersion + revision = jumpstarterRevision + } else { + version = ociVersion + revision = ociRevision + } + + // Return labels even if only partially available (version OR revision can be empty) + return &container.ImageLabels{ + Version: version, + Revision: revision, + }, nil +} + // RunHostCommand implements the HostManager interface by exposing runCommand func (m *SSHHostManager) RunHostCommand(command string) (*CommandResult, error) { return m.runCommand(command) From 77c725c7e0fcd6df5b3ba76555a00be4520a91cf Mon Sep 17 00:00:00 2001 From: Michal Skrivanek Date: Fri, 3 Oct 2025 18:40:41 +0200 Subject: [PATCH 2/2] add forgotten test --- internal/exporter/host/host_test.go | 224 ++++++++++++++++++++++++++++ 1 file changed, 224 insertions(+) create mode 100644 internal/exporter/host/host_test.go diff --git a/internal/exporter/host/host_test.go b/internal/exporter/host/host_test.go new file mode 100644 index 0000000..cf76316 --- /dev/null +++ b/internal/exporter/host/host_test.go @@ -0,0 +1,224 @@ +package host + +import ( + "testing" + + "github.com/jumpstarter-dev/jumpstarter-lab-config/api/v1alpha1" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestDeadAnnotationFiltering(t *testing.T) { + tests := []struct { + name string + exporterInstances []*v1alpha1.ExporterInstance + expectedAliveInstances int + expectedDeadInstances int + }{ + { + name: "no dead instances", + exporterInstances: []*v1alpha1.ExporterInstance{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "instance1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "instance2", + }, + }, + }, + expectedAliveInstances: 2, + expectedDeadInstances: 0, + }, + { + name: "one dead instance", + exporterInstances: []*v1alpha1.ExporterInstance{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "instance1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "instance2", + Annotations: map[string]string{ + "dead": "true", + }, + }, + }, + }, + expectedAliveInstances: 1, + expectedDeadInstances: 1, + }, + { + name: "all dead instances", + exporterInstances: []*v1alpha1.ExporterInstance{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "instance1", + Annotations: map[string]string{ + "dead": "true", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "instance2", + Annotations: map[string]string{ + "dead": "true", + }, + }, + }, + }, + expectedAliveInstances: 0, + expectedDeadInstances: 2, + }, + { + name: "dead annotation with false value", + exporterInstances: []*v1alpha1.ExporterInstance{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "instance1", + Annotations: map[string]string{ + "dead": "false", + }, + }, + }, + }, + expectedAliveInstances: 0, + expectedDeadInstances: 1, + }, + { + name: "dead annotation with other value", + exporterInstances: []*v1alpha1.ExporterInstance{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "instance1", + Annotations: map[string]string{ + "dead": "maybe", + }, + }, + }, + }, + expectedAliveInstances: 0, + expectedDeadInstances: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Simulate the filtering logic from SyncExporterHosts + aliveInstances := []*v1alpha1.ExporterInstance{} + deadCount := 0 + + for _, exporterInstance := range tt.exporterInstances { + if _, exists := exporterInstance.Annotations["dead"]; exists { + deadCount++ + } else { + aliveInstances = append(aliveInstances, exporterInstance) + } + } + + assert.Equal(t, tt.expectedAliveInstances, len(aliveInstances), "unexpected number of alive instances") + assert.Equal(t, tt.expectedDeadInstances, deadCount, "unexpected number of dead instances") + }) + } +} + +func TestHostSkippingBehavior(t *testing.T) { + tests := []struct { + name string + exporterInstances []*v1alpha1.ExporterInstance + shouldSkipHost bool + expectedAlivCount int + description string + }{ + { + name: "mixed dead and alive instances - should NOT skip host", + exporterInstances: []*v1alpha1.ExporterInstance{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "alive-instance", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "dead-instance", + Annotations: map[string]string{ + "dead": "true", + }, + }, + }, + }, + shouldSkipHost: false, // Host should be processed because there's an alive instance + expectedAlivCount: 1, + description: "When there are both dead and alive instances, host should be processed", + }, + { + name: "all instances dead - should SKIP host", + exporterInstances: []*v1alpha1.ExporterInstance{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "dead-instance-1", + Annotations: map[string]string{ + "dead": "true", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "dead-instance-2", + Annotations: map[string]string{ + "dead": "true", + }, + }, + }, + }, + shouldSkipHost: true, // Host should be skipped because all instances are dead + expectedAlivCount: 0, + description: "When all instances are dead, host should be skipped entirely", + }, + { + name: "all instances alive - should NOT skip host", + exporterInstances: []*v1alpha1.ExporterInstance{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "alive-instance-1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "alive-instance-2", + }, + }, + }, + shouldSkipHost: false, // Host should be processed because all instances are alive + expectedAlivCount: 2, + description: "When all instances are alive, host should be processed normally", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Simulate the new logic from SyncExporterHosts + // Check if all instances are dead + allDead := true + aliveCount := 0 + + for _, exporterInstance := range tt.exporterInstances { + if _, exists := exporterInstance.Annotations["dead"]; !exists { + allDead = false + aliveCount++ + } + } + + // Host is skipped only if all instances are dead (and there are instances) + hostWouldBeSkipped := len(tt.exporterInstances) > 0 && allDead + + assert.Equal(t, tt.shouldSkipHost, hostWouldBeSkipped, tt.description) + assert.Equal(t, tt.expectedAlivCount, aliveCount, "unexpected number of alive instances") + }) + } +}