ARO-26764: promote OCM label prefix and validation skip reason to API package

api/hypershift/v1beta1/hostedcluster_conditions.go

@@ -285,6 +285,7 @@ const (
 	ProxyCABundleInvalidReason            = "ProxyCABundleInvalid"
 	PlatformCredentialsNotFoundReason     = "PlatformCredentialsNotFound"
 	InvalidImageReason                    = "InvalidImage"
+	ReleaseImageValidationSkippedReason   = "ReleaseImageValidationSkipped"
 	InvalidIdentityProvider               = "InvalidIdentityProvider"
 	PayloadArchNotFoundReason             = "PayloadArchNotFound"
 

api/hypershift/v1beta1/hostedcluster_types.go

@@ -188,6 +188,10 @@ const (
 	// resource-request-override.hypershift.openshift.io/kube-apiserver.kube-apiserver: memory=3Gi,cpu=2000m
 	ResourceRequestOverrideAnnotationPrefix = "resource-request-override.hypershift.openshift.io"
 
+	// OCMLabelPrefix is the label key prefix used by OCM to tag HostedCluster
+	// resources. Labels with this prefix are mirrored to the HostedControlPlane.
+	OCMLabelPrefix = "api.openshift.com/"
+
 	// LimitedSupportLabel is a label that can be used by consumers to indicate
 	// a cluster is somehow out of regular support policy.
 	// https://docs.openshift.com/rosa/rosa_architecture/rosa_policy_service_definition/rosa-service-definition.html#rosa-limited-support_rosa-service-definition.

hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go

@@ -30,7 +30,6 @@ import (
 	"time"
 
 	hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
-	hyperkarpenterv1 "github.com/openshift/hypershift/api/karpenter/v1"
 	"github.com/openshift/hypershift/api/util/configrefs"
 	"github.com/openshift/hypershift/cmd/util"
 	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/imageprovider"
@@ -227,7 +226,7 @@ func (r *HostedClusterReconciler) SetupWithManager(mgr ctrl.Manager, createOrUpd
 	// namespaces, the events are filtered to enqueue only those resources which
 	// are annotated as being associated with a hostedcluster (using an annotation).
 	bldr := ctrl.NewControllerManagedBy(mgr).
-		For(&hyperv1.HostedCluster{}, builder.WithPredicates(hyperutil.PredicatesForHostedClusterAnnotationScoping(mgr.GetClient()))).
+		For(&hyperv1.HostedCluster{}, builder.WithPredicates(hostedClusterPrimaryPredicate(mgr.GetClient()))).
 		WithOptions(controller.Options{
 			RateLimiter:             workqueue.NewTypedItemExponentialFailureRateLimiter[reconcile.Request](1*time.Second, 10*time.Second),
 			MaxConcurrentReconciles: 10,
@@ -1188,13 +1187,22 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques
 		// This check can be expensive looking up release image versions
 		// (hopefully they are cached).  Skip if we have already observed for
 		// this generation.
-		if condition == nil || condition.ObservedGeneration != hcluster.Generation || condition.Status != metav1.ConditionTrue {
+		if shouldValidateReleaseImage(hcluster, condition) {
 			condition := metav1.Condition{
 				Type:               string(hyperv1.ValidReleaseImage),
 				ObservedGeneration: hcluster.Generation,
 			}
-			err := r.validateReleaseImage(ctx, hcluster, releaseProvider)
-			if err != nil {
+			skipReleaseImageValidation := hasSkipReleaseImageValidationAnnotation(hcluster)
+			var err error
+			if !skipReleaseImageValidation {
+				err = r.validateReleaseImage(ctx, hcluster, releaseProvider)
+			}
+			switch {
+			case skipReleaseImageValidation:
+				condition.Status = metav1.ConditionTrue
+				condition.Message = "Release image validation is skipped by annotation"
+				condition.Reason = hyperv1.ReleaseImageValidationSkippedReason
+			case err != nil:
 				condition.Status = metav1.ConditionFalse
 				condition.Message = err.Error()
 
@@ -1203,7 +1211,7 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques
 				} else {
 					condition.Reason = hyperv1.InvalidImageReason
 				}
-			} else {
+			default:
 				condition.Status = metav1.ConditionTrue
 				condition.Message = "Release image is valid"
 				condition.Reason = hyperv1.AsExpectedReason
@@ -2318,57 +2326,7 @@ func reconcileHostedControlPlaneAnnotations(hcp *hyperv1.HostedControlPlane, hcl
 	hcp.Annotations[k8sutil.HostedClusterAnnotation] = client.ObjectKeyFromObject(hcluster).String()
 
 	// These annotations are copied from the HostedCluster
-	mirroredAnnotations := []string{
-		hyperv1.DisablePKIReconciliationAnnotation,
-		hyperv1.OauthLoginURLOverrideAnnotation,
-		hyperv1.KonnectivityAgentImageAnnotation,
-		hyperv1.KonnectivityServerImageAnnotation,
-		hyperv1.ClusterAutoscalerImage,
-		hyperv1.IBMCloudKMSProviderImage,
-		hyperv1.AWSKMSProviderImage,
-		hyperv1.PortierisImageAnnotation,
-		k8sutil.DebugDeploymentsAnnotation,
-		hyperv1.DisableProfilingAnnotation,
-		hyperv1.PrivateIngressControllerAnnotation,
-		hyperv1.IngressControllerLoadBalancerScope,
-		hyperv1.CleanupCloudResourcesAnnotation,
-		hyperv1.ControlPlanePriorityClass,
-		hyperv1.APICriticalPriorityClass,
-		hyperv1.EtcdPriorityClass,
-		hyperv1.EnsureExistsPullSecretReconciliation,
-		hyperv1.TopologyAnnotation,
-		hyperv1.DisableMachineManagement,
-		hyperv1.CertifiedOperatorsCatalogImageAnnotation,
-		hyperv1.CommunityOperatorsCatalogImageAnnotation,
-		hyperv1.RedHatMarketplaceCatalogImageAnnotation,
-		hyperv1.RedHatOperatorsCatalogImageAnnotation,
-		hyperv1.OLMCatalogsISRegistryOverridesAnnotation,
-		hyperv1.KubeAPIServerGOGCAnnotation,
-		hyperv1.KubeAPIServerGOMemoryLimitAnnotation,
-		hyperv1.RequestServingNodeAdditionalSelectorAnnotation,
-		hyperv1.AWSLoadBalancerSubnetsAnnotation,
-		hyperv1.AWSLoadBalancerTargetNodesAnnotation,
-		hyperv1.AWSLoadBalancerHealthProbeModeAnnotation,
-		hyperv1.AzureLoadBalancerHealthProbeModeAnnotation,
-		hyperv1.SharedLoadBalancerHealthProbePathAnnotation,
-		hyperv1.SharedLoadBalancerHealthProbePortAnnotation,
-		hyperv1.ManagementPlatformAnnotation,
-		hyperv1.KubeAPIServerVerbosityLevelAnnotation,
-		hyperv1.KubeAPIServerMaximumRequestsInFlight,
-		hyperv1.KubeAPIServerMaximumMutatingRequestsInFlight,
-		hyperv1.DisableIgnitionServerAnnotation,
-		hyperv1.AWSMachinePublicIPs,
-		hyperv1.AWSKarpenterDefaultInstanceProfile,
-		hyperkarpenterv1.KarpenterProviderAWSImage,
-		hyperv1.KubeAPIServerGoAwayChance,
-		hyperv1.KubeAPIServerServiceAccountTokenMaxExpiration,
-		hyperv1.HostedClusterRestoredFromBackupAnnotation,
-		// TODO: Remove this once the the input is in the HostedCluster AWS API.
-		"hypershift.openshift.io/aws-termination-handler-queue-url",
-		hyperv1.SwiftPodNetworkInstanceAnnotation,
-		hyperv1.EnableMetricsForwarding,
-	}
-	for _, key := range mirroredAnnotations {
+	for _, key := range hostedClusterMirroredAnnotations {
 		val, hasVal := hcluster.Annotations[key]
 		if hasVal {
 			hcp.Annotations[key] = val
@@ -2377,22 +2335,17 @@ func reconcileHostedControlPlaneAnnotations(hcp *hyperv1.HostedControlPlane, hcl
 		}
 	}
 
-	prefixesToSync := []string{
-		hyperv1.IdentityProviderOverridesAnnotationPrefix,
-		hyperv1.ResourceRequestOverrideAnnotationPrefix,
-	}
-
 	// All annotations on the HostedCluster with prefixes to sync
 	// should be synced
 	for key := range hcp.Annotations {
-		for _, prefix := range prefixesToSync {
+		for _, prefix := range hostedClusterActionableAnnotationPrefixes {
 			if strings.HasPrefix(key, prefix) {
 				delete(hcp.Annotations, key)
 			}
 		}
 	}
 	for key, val := range hcluster.Annotations {
-		for _, prefix := range prefixesToSync {
+		for _, prefix := range hostedClusterActionableAnnotationPrefixes {
 			if strings.HasPrefix(key, prefix) {
 				hcp.Annotations[key] = val
 			}
@@ -2445,9 +2398,20 @@ func reconcileHostedControlPlane(hcp *hyperv1.HostedControlPlane, hcluster *hype
 	}
 	// All labels on the HostedCluster with this special prefix are copied
 	// Those are labels set by OCM
+	for key := range hcp.Labels {
+		for _, prefix := range hostedClusterActionableLabelPrefixes {
+			if strings.HasPrefix(key, prefix) {
+				delete(hcp.Labels, key)
+				break
+			}
+		}
+	}
 	for key, val := range hcluster.Labels {
-		if strings.HasPrefix(key, "api.openshift.com") {
-			hcp.Labels[key] = val
+		for _, prefix := range hostedClusterActionableLabelPrefixes {
+			if strings.HasPrefix(key, prefix) {
+				hcp.Labels[key] = val
+				break
+			}
 		}
 	}
 
@@ -3743,7 +3707,7 @@ func (r *HostedClusterReconciler) validateUserCAConfigMaps(ctx context.Context,
 }
 
 func (r *HostedClusterReconciler) validateReleaseImage(ctx context.Context, hc *hyperv1.HostedCluster, releaseProvider releaseinfo.ProviderWithOpenShiftImageRegistryOverrides) error {
-	if _, exists := hc.Annotations[hyperv1.SkipReleaseImageValidation]; exists {
+	if hasSkipReleaseImageValidationAnnotation(hc) {
 		return nil
 	}
 	pullSecretBytes, err := hyperutil.GetPullSecretBytes(ctx, r.Client, hc)
@@ -3778,6 +3742,24 @@ func (r *HostedClusterReconciler) validateReleaseImage(ctx context.Context, hc *
 	return supportedversion.IsValidReleaseVersion(&version, currentVersion, &supportedversion.LatestSupportedVersion, &minSupportedVersion, hc.Spec.Networking.NetworkType, hc.Spec.Platform.Type)
 }
 
+func shouldValidateReleaseImage(hcluster *hyperv1.HostedCluster, condition *metav1.Condition) bool {
+	if condition == nil || condition.ObservedGeneration != hcluster.Generation || condition.Status != metav1.ConditionTrue {
+		return true
+	}
+
+	skipReleaseImageValidation := hasSkipReleaseImageValidationAnnotation(hcluster)
+	if skipReleaseImageValidation {
+		return condition.Reason != hyperv1.ReleaseImageValidationSkippedReason
+	}
+
+	return condition.Reason == hyperv1.ReleaseImageValidationSkippedReason
+}
+
+func hasSkipReleaseImageValidationAnnotation(hcluster *hyperv1.HostedCluster) bool {
+	_, exists := hcluster.Annotations[hyperv1.SkipReleaseImageValidation]
+	return exists
+}
+
 func isProgressing(hc *hyperv1.HostedCluster, releaseImage *releaseinfo.ReleaseImage, refWithDigest func() (string, error)) (bool, error) {
 	for _, condition := range hc.Status.Conditions {
 		switch string(condition.Type) {

hypershift-operator/controllers/hostedcluster/hostedcluster_hcp_labels_test.go

@@ -0,0 +1,45 @@
+package hostedcluster
+
+import (
+	"testing"
+
+	hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestReconcileHostedControlPlane_WhenActionableLabelsAreRemovedItShouldClearStaleHostedControlPlaneLabels(t *testing.T) {
+	t.Parallel()
+
+	hcluster := &hyperv1.HostedCluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "example",
+			Namespace: "clusters",
+			Labels:    map[string]string{},
+		},
+	}
+
+	hcp := &hyperv1.HostedControlPlane{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "example",
+			Namespace: "clusters-example",
+			Labels: map[string]string{
+				"api.openshift.com/id": "stale",
+				"other":                "keep",
+			},
+		},
+	}
+
+	if err := reconcileHostedControlPlane(hcp, hcluster, false, false, func() (map[string]string, error) {
+		return map[string]string{}, nil
+	}); err != nil {
+		t.Fatalf("reconcileHostedControlPlane returned error: %v", err)
+	}
+
+	if _, exists := hcp.Labels["api.openshift.com/id"]; exists {
+		t.Fatal("expected stale api.openshift.com label to be removed from hosted control plane")
+	}
+	if value := hcp.Labels["other"]; value != "keep" {
+		t.Fatalf("expected unrelated label to be preserved, got %q", value)
+	}
+}