[release-4.16] Resync release-4.16 branch with main 2024-05-21

.ci-operator.yaml

@@ -1,4 +1,4 @@
 build_root_image:
   name: release
   namespace: openshift
-  tag: rhel-8-release-golang-1.20-openshift-4.14
+  tag: rhel-8-release-golang-1.21-openshift-4.16

.testcoverage.yml

@@ -0,0 +1,50 @@
+# (mandatory)
+# Path to coverprofile file (output of `go test -coverprofile` command).
+#
+# For cases where there are many coverage profiles, such as when running
+# unit tests and integration tests separately, you can combine all those
+# profiles into one. In this case, the profile should have a comma-separated list
+# of profile files, e.g., 'cover_unit.out,cover_integration.out'.
+profile: cover.out
+
+# (optional; but recommended to set)
+# When specified reported file paths will not contain local prefix in the output
+local-prefix: "github.com/openshift-kni/lifecycle-agent"
+
+# Holds coverage thresholds percentages, values should be in range [0-100]
+threshold:
+  # (optional; default 0)
+  # The minimum coverage that each file should have
+  file: 0
+
+  # (optional; default 0)
+  # The minimum coverage that each package should have
+  package: 0
+
+  # (optional; default 0)
+  # The minimum total coverage project should have
+  total: 30
+
+# Holds regexp rules which will override thresholds for matched files or packages
+# using their paths.
+#
+# First rule from this list that matches file or package is going to apply
+# new threshold to it. If project has multiple rules that match same path,
+# override rules should be listed in order from specific to more general rules.
+override:
+  # Increase coverage threshold to 100% for `foo` package
+  # (default is 80, as configured above in this example)
+  - threshold: 100
+    path: ^pkg/lib/foo$
+
+# Holds regexp rules which will exclude matched files or packages
+# from coverage statistics
+exclude:
+  # Exclude files or packages matching their paths
+  paths:
+    - \.pb\.go$    # excludes all protobuf generated files
+    - ^pkg/bar     # exclude package `pkg/bar`
+
+# NOTES:
+# - symbol `/` in all path regexps will be replaced by current OS file path separator
+#   to properly work on Windows

Dockerfile

@@ -1,6 +1,6 @@
 #####################################################################################################
 # Build the binaries
-FROM registry.access.redhat.com/ubi9/go-toolset:1.20 as builder
+FROM registry.access.redhat.com/ubi9/go-toolset:1.21 as builder
 
 # Bring in the go dependencies before anything else so we can take
 # advantage of caching these layers in future builds.

Makefile

@@ -8,6 +8,9 @@ VERSION ?= 4.16.0
 # You can use podman or docker as a container engine. Notice that there are some options that might be only valid for one of them.
 ENGINE ?= docker
 
+# Path to go binary in local
+GOBIN ?= $$(go env GOPATH)/bin
+
 # CHANNELS define the bundle channels used in the bundle.
 # Add a new line here if you would like to change its default config. (E.g CHANNELS = "preview,fast,stable")
 # To re-generate a bundle for other specific channels without changing the standard setup, you can:
@@ -125,6 +128,15 @@ else
 	$(OPERATOR_SDK) scorecard bundle
 endif
 
+.PHONY: install-go-test-coverage
+install-go-test-coverage:
+	go install github.com/vladopajic/go-test-coverage/v2@latest
+
+.PHONY: check-coverage
+check-coverage: install-go-test-coverage
+	go test ./... -coverprofile=./cover.out -covermode=atomic -coverpkg=./...
+	${GOBIN}/go-test-coverage --config=./.testcoverage.yml
+
 .PHONY: ci-job
 ci-job: common-deps-update generate fmt vet golangci-lint unittest shellcheck bashate bundle-check
 

README.md

@@ -1,5 +1,9 @@
 # Lifecycle Agent Operator
 
+[![Go Report Card](https://goreportcard.com/badge/github.com/openshift-kni/lifecycle-agent)](https://goreportcard.com/report/github.com/openshift-kni/lifecycle-agent)
+[![Go Reference](https://pkg.go.dev/badge/github.com/openshift-kni/lifecycle-agent.svg)](https://pkg.go.dev/github.com/openshift-kni/lifecycle-agent)
+[![License Apache](https://img.shields.io/github/license/openshift-kni/lifecycle-agent)](https://opensource.org/licenses/Apache-2.0)
+
 ## What it is
 
 The Lifecycle Agent Operator provides local (on cluster) lifecycle management services for Single

config/manifests/bases/lifecycle-agent.clusterserviceversion.yaml

@@ -11,8 +11,8 @@ metadata:
     features.operators.openshift.io/cni: "false"
     features.operators.openshift.io/csi: "false"
     features.operators.openshift.io/disconnected: "true"
-    features.operators.openshift.io/fips-compliant: "false"
-    features.operators.openshift.io/proxy-aware: "false"
+    features.operators.openshift.io/fips-compliant: "true"
+    features.operators.openshift.io/proxy-aware: "true"
     features.operators.openshift.io/tls-profiles: "false"
     features.operators.openshift.io/token-auth-aws: "false"
     features.operators.openshift.io/token-auth-azure: "false"

config/rbac/role.yaml

@@ -28,6 +28,7 @@ rules:
   resources:
   - namespaces
   verbs:
+  - create
   - delete
   - get
   - list

controllers/idle_handlers.go

@@ -21,10 +21,11 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/openshift-kni/lifecycle-agent/internal/prep"
+
 	"github.com/go-logr/logr"
 	"github.com/openshift-kni/lifecycle-agent/internal/extramanifest"
 	"github.com/openshift-kni/lifecycle-agent/internal/ostreeclient"
-	"github.com/openshift-kni/lifecycle-agent/internal/prep"
 	"github.com/openshift-kni/lifecycle-agent/lca-cli/ops"
 	rpmostreeclient "github.com/openshift-kni/lifecycle-agent/lca-cli/ostreeclient"
 
@@ -152,21 +153,17 @@ func (r *ImageBasedUpgradeReconciler) cleanup(ctx context.Context, ibu *ibuv1.Im
 	}
 
 	r.Log.Info("Cleaning up stateroot")
-	if err := CleanupUnbootedStateroots(r.Log, r.Ops, r.OstreeClient, r.RPMOstreeClient); err != nil {
-		handleError(err, "failed to cleanup stateroots.")
-	}
-	r.Log.Info("Cleaning up stateroot setup job")
-	err := prep.DeleteStaterootSetupJob(ctx, r.Client, r.Log)
-	if err != nil {
-		handleError(err, "failed to cleanup stateroots setup job.")
+	if err := r.cleanupStateroot(ctx); err != nil {
+		handleError(err, "failed to cleanup stateroot")
 	}
 
 	r.Log.Info("Cleaning up precache")
 	if err := r.Precache.Cleanup(ctx); err != nil {
 		handleError(err, "failed to cleanup precaching resources.")
 	}
 
-	if err := extramanifest.RemoveAnnotationWarnUnknownCRD(r.Client, ibu, r.Log); err != nil {
+	r.Log.Info("Removing annotation with warning")
+	if err := extramanifest.RemoveAnnotationEMWarningValidation(r.Client, r.Log, ibu); err != nil {
 		handleError(err, "failed to remove extra manifest warning annotation from IBU")
 	}
 
@@ -189,6 +186,21 @@ func (r *ImageBasedUpgradeReconciler) cleanup(ctx context.Context, ibu *ibuv1.Im
 	return successful, errorMessage
 }
 
+func (r *ImageBasedUpgradeReconciler) cleanupStateroot(ctx context.Context) error {
+	r.Log.Info("Cleaning up cluster stateroot resources")
+	if err := prep.DeleteStaterootSetupJob(ctx, r.Client, r.Log); err != nil {
+		return fmt.Errorf("failed to cleanup cluster stateroot resources: %w", err)
+	}
+
+	r.Log.Info("Cleaning up unbooted stateroot resources")
+	if err := CleanupUnbootedStateroots(r.Log, r.Ops, r.OstreeClient, r.RPMOstreeClient); err != nil {
+		return fmt.Errorf("failed to clean up host stateroot resources: %w", err)
+	}
+
+	r.Log.Info("Successfully cleaned all resources related to stateroot setup")
+	return nil
+}
+
 func cleanupIBUFiles() error {
 	if _, err := os.Stat(common.PathOutsideChroot(utils.IBUWorkspacePath)); err != nil {
 		return nil
@@ -247,7 +259,7 @@ func CleanupUnbootedStateroots(log logr.Logger, ops ops.Ops, ostreeClient ostree
 	}
 
 	if failures == 0 {
-		log.Info("Stateroot cleanup successfully")
+		log.Info("Unbooted stateroot cleanup completed successfully")
 		return nil
 	}
 	return fmt.Errorf("failed to remove %d stateroots", failures)

controllers/prep_handlers.go

@@ -96,6 +96,14 @@ func GetSeedImage(c client.Client, ctx context.Context, ibu *ibuv1.ImageBasedUpg
 		seedHasProxy = seedInfo.HasProxy
 	}
 
+	seedHasFIPS := false
+	if seedInfo != nil {
+		// Older images may not have the seed cluster info label, in which case
+		// we assume no FIPS so that if the current cluster has FIPS, it will
+		// fail the compatibility check.
+		seedHasFIPS = seedInfo.HasFIPS
+	}
+
 	clusterHasProxy, err := lcautils.HasProxy(ctx, c)
 	if err != nil {
 		return fmt.Errorf("failed to check if cluster has proxy: %w", err)
@@ -106,6 +114,16 @@ func GetSeedImage(c client.Client, ctx context.Context, ibu *ibuv1.ImageBasedUpg
 		return fmt.Errorf("checking seed image compatibility: %w", err)
 	}
 
+	clusterHasFIPS, err := lcautils.HasFIPS(ctx, c)
+	if err != nil {
+		return fmt.Errorf("failed to check if cluster has fips: %w", err)
+	}
+
+	log.Info("Checking seed image FIPS compatibility")
+	if err := checkSeedImageFIPSCompatibility(seedHasFIPS, clusterHasFIPS); err != nil {
+		return fmt.Errorf("checking seed image compatibility: %w", err)
+	}
+
 	return nil
 }
 
@@ -196,6 +214,23 @@ func checkSeedImageProxyCompatibility(seedHasProxy, hasProxy bool) error {
 	return nil
 }
 
+// checkSeedImageFIPSCompatibility checks for FIPS configuration compatibility
+// of the seed image vs the current cluster. If the seed image has FIPS enabled
+// and the cluster being upgraded doesn't, we cannot proceed as recert does not
+// support FIPS rename under those conditions. Similarly, we cannot proceed if
+// the cluster being upgraded has FIPS but the seed image doesn't.
+func checkSeedImageFIPSCompatibility(seedHasFIPS, hasFIPS bool) error {
+	if seedHasFIPS && !hasFIPS {
+		return fmt.Errorf("seed image has FIPS enabled but the cluster being upgraded does not, this combination is not supported")
+	}
+
+	if !seedHasFIPS && hasFIPS {
+		return fmt.Errorf("seed image does not have FIPS enabled but the cluster being upgraded does, this combination is not supported")
+	}
+
+	return nil
+}
+
 // validateSeedOcpVersion rejects upgrade request if seed image version is not higher than current cluster (target) OCP version
 func (r *ImageBasedUpgradeReconciler) validateSeedOcpVersion(seedOcpVersion string) error {
 	// get target OCP version
@@ -286,6 +321,13 @@ func (r *ImageBasedUpgradeReconciler) launchPrecaching(ctx context.Context, imag
 	return nil
 }
 
+func getSeedManifestPath(osname string) string {
+	return filepath.Join(
+		common.GetStaterootPath(osname),
+		filepath.Join(common.SeedDataDir, common.SeedClusterInfoFileName),
+	)
+}
+
 // validateIBUSpec validates the fields in the IBU spec
 func (r *ImageBasedUpgradeReconciler) validateIBUSpec(ctx context.Context, ibu *ibuv1.ImageBasedUpgrade) error {
 	// Check spec against this cluster's version and possibly exit early
@@ -309,9 +351,16 @@ func (r *ImageBasedUpgradeReconciler) validateIBUSpec(ctx context.Context, ibu *
 
 	// Validate the extraManifests configmap if it's provided
 	if len(ibu.Spec.ExtraManifests) != 0 {
-		if err := r.ExtraManifest.ValidateExtraManifestConfigmaps(ctx, ibu.Spec.ExtraManifests, ibu); err != nil {
+		warn, err := r.ExtraManifest.ValidateExtraManifestConfigmaps(ctx, ibu.Spec.ExtraManifests)
+		if err != nil {
 			return fmt.Errorf("failed to validate extramanifest cms: %w", err)
 		}
+		if warn != "" {
+			r.Log.Info(fmt.Sprintf("Adding IBU annotation '%s' with the extramanifest validation warning", extramanifest.ValidationWarningAnnotation))
+			if err := extramanifest.AddAnnotationEMWarningValidation(r.Client, r.Log, ibu, warn); err != nil {
+				return fmt.Errorf("failed to add extramanifest warning validation annotation: %w", err)
+			}
+		}
 	}
 
 	// Validate the manifests from policies if related annotations are specified
@@ -376,26 +425,26 @@ func (r *ImageBasedUpgradeReconciler) handlePrep(ctx context.Context, ibu *ibuv1
 			if _, err := prep.LaunchStaterootSetupJob(ctx, r.Client, ibu, r.Scheme, r.Log); err != nil {
 				return prepFailDoNotRequeue(r.Log, fmt.Sprintf("failed launch stateroot job: %s", err.Error()), ibu)
 			}
-			return prepInProgressRequeue(r.Log, fmt.Sprintf("Successfully launched a new job `%s` in namespace `%s`", prep.JobName, common.LcaNamespace), ibu)
+			return prepInProgressRequeue(r.Log, fmt.Sprintf("Successfully launched a new job for stateroot setup. %s", getJobMetadataString(staterootSetupJob)), ibu)
 		}
 		return prepFailDoNotRequeue(r.Log, fmt.Sprintf("failed to get stateroot setup job: %s", err.Error()), ibu)
 	}
 
 	r.Log.Info("Verifying stateroot setup job status")
 
-	// job deletion not allowed
+	// job deletion is not allowed
 	if staterootSetupJob.GetDeletionTimestamp() != nil {
-		return prepFailDoNotRequeue(r.Log, "stateroot job is marked to be deleted, this is not allowed", ibu)
+		return prepFailDoNotRequeue(r.Log, fmt.Sprintf("stateroot job is marked to be deleted, this is not allowed. %s", getJobMetadataString(staterootSetupJob)), ibu)
 	}
 
 	// check .status
 	_, staterootSetupFinishedType := common.IsJobFinished(staterootSetupJob)
 	switch staterootSetupFinishedType {
 	case "":
 		common.LogPodLogs(staterootSetupJob, r.Log, r.Clientset)
-		return prepInProgressRequeue(r.Log, "Stateroot setup in progress", ibu)
+		return prepInProgressRequeue(r.Log, fmt.Sprintf("Stateroot setup job in progress. %s", getJobMetadataString(staterootSetupJob)), ibu)
 	case kbatch.JobFailed:
-		return prepFailDoNotRequeue(r.Log, fmt.Sprintf("stateroot setup job could not complete. Please check job logs for more, job_name: %s, job_ns: %s", staterootSetupJob.GetName(), staterootSetupJob.GetNamespace()), ibu)
+		return prepFailDoNotRequeue(r.Log, fmt.Sprintf("stateroot setup job failed to complete. %s", getJobMetadataString(staterootSetupJob)), ibu)
 	case kbatch.JobComplete:
 		r.Log.Info("Stateroot job completed successfully", "completion time", staterootSetupJob.Status.CompletionTime, "total time", staterootSetupJob.Status.CompletionTime.Sub(staterootSetupJob.Status.StartTime.Time))
 	}
@@ -408,26 +457,26 @@ func (r *ImageBasedUpgradeReconciler) handlePrep(ctx context.Context, ibu *ibuv1
 			if err := r.launchPrecaching(ctx, precache.ImageListFile, ibu); err != nil {
 				return prepFailDoNotRequeue(r.Log, fmt.Sprintf("failed to launch precaching job: %s", err.Error()), ibu)
 			}
-			return prepInProgressRequeue(r.Log, fmt.Sprintf("Successfully launched a new job `%s` in namespace `%s`", precache.LcaPrecacheJobName, common.LcaNamespace), ibu)
+			return prepInProgressRequeue(r.Log, fmt.Sprintf("Successfully launched a new job precache. %s", getJobMetadataString(precacheJob)), ibu)
 		}
 		return prepFailDoNotRequeue(r.Log, fmt.Sprintf("failed to get precache job: %s", err.Error()), ibu)
 	}
 
 	r.Log.Info("Verifying precache job status")
 
-	// job deletion not allowed
+	// job deletion is not allowed
 	if precacheJob.GetDeletionTimestamp() != nil {
-		return prepFailDoNotRequeue(r.Log, "precache job is marked to be deleted, this not allowed", ibu)
+		return prepFailDoNotRequeue(r.Log, fmt.Sprintf("precache job is marked to be deleted, this not allowed. %s", getJobMetadataString(precacheJob)), ibu)
 	}
 
 	// check .status
 	_, precacheFinishedType := common.IsJobFinished(precacheJob)
 	switch precacheFinishedType {
 	case "":
 		common.LogPodLogs(precacheJob, r.Log, r.Clientset) // pod logs
-		return prepInProgressRequeue(r.Log, fmt.Sprintf("Precache job in progress: %s", precache.GetPrecacheStatusFileContent()), ibu)
+		return prepInProgressRequeue(r.Log, fmt.Sprintf("Precache job in progress. %s. %s", getJobMetadataString(precacheJob), precache.GetPrecacheStatusFileContent()), ibu)
 	case kbatch.JobFailed:
-		return prepFailDoNotRequeue(r.Log, fmt.Sprintf("precache job could not complete. Please check job logs for more, job_name: %s, job_ns: %s", precacheJob.GetName(), precacheJob.GetNamespace()), ibu)
+		return prepFailDoNotRequeue(r.Log, fmt.Sprintf("precache job failed to complete. %s", getJobMetadataString(precacheJob)), ibu)
 	case kbatch.JobComplete:
 		r.Log.Info("Precache job completed successfully", "completion time", precacheJob.Status.CompletionTime, "total time", precacheJob.Status.CompletionTime.Sub(precacheJob.Status.StartTime.Time))
 	}
@@ -446,6 +495,10 @@ func prepFailDoNotRequeue(log logr.Logger, msg string, ibu *ibuv1.ImageBasedUpgr
 // prepInProgressRequeue helper function when everything is a success at the end
 func prepSuccessDoNotRequeue(log logr.Logger, ibu *ibuv1.ImageBasedUpgrade) (ctrl.Result, error) {
 	msg := "Prep stage completed successfully"
+	if _, exists := ibu.GetAnnotations()[extramanifest.ValidationWarningAnnotation]; exists {
+		msg = fmt.Sprintf("Prep stage completed with extramanifests validation warning. Please check the annotation '%s' for details.", extramanifest.ValidationWarningAnnotation)
+	}
+
 	log.Info(msg)
 	utils.SetPrepStatusCompleted(ibu, msg)
 	return doNotRequeue(), nil
@@ -459,9 +512,10 @@ func prepInProgressRequeue(log logr.Logger, msg string, ibu *ibuv1.ImageBasedUpg
 	return requeueWithShortInterval(), nil
 }
 
-func getSeedManifestPath(osname string) string {
-	return filepath.Join(
-		common.GetStaterootPath(osname),
-		filepath.Join(common.SeedDataDir, common.SeedClusterInfoFileName),
-	)
+// getJobMetadataString a helper to append job metadata for helpful logs
+func getJobMetadataString(job *kbatch.Job) string {
+	if job == nil {
+		return "job is nil"
+	}
+	return fmt.Sprintf("job-name: %s, job-namespace: %s", job.GetName(), job.GetNamespace())
 }