From 38d1840629372bdab0c8622c0c329760e8fb6084 Mon Sep 17 00:00:00 2001 From: Alessandro Affinito Date: Tue, 5 May 2026 16:26:18 +0200 Subject: [PATCH 01/11] fix(hostedcluster): narrow primary watch to actionable metadata updates Reduce self-induced HostedCluster reconciles by filtering the primary watch to spec changes and explicit metadata triggers that already affect reconciliation. Preserve mirrored annotation and label behavior with focused falsification tests so stale queued requests still reconcile the latest HostedCluster state. Signed-off-by: Alessandro Affinito Commit-Message-Assisted-by: Claude (via Claude Code) Co-authored-by: Cursor --- .../hostedcluster/hostedcluster_controller.go | 69 +--- .../hostedcluster_hcp_labels_test.go | 45 +++ .../hostedcluster/hostedcluster_predicates.go | 203 ++++++++++ .../hostedcluster_predicates_test.go | 380 ++++++++++++++++++ ...edcluster_reconciliation_condition_test.go | 227 +++++++++++ 5 files changed, 864 insertions(+), 60 deletions(-) create mode 100644 hypershift-operator/controllers/hostedcluster/hostedcluster_hcp_labels_test.go create mode 100644 hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go create mode 100644 hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go create mode 100644 hypershift-operator/controllers/hostedcluster/hostedcluster_reconciliation_condition_test.go diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index 022b442d182..162bbe37d18 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/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, @@ -2318,57 +2317,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 +2326,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,6 +2389,11 @@ 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 { + if strings.HasPrefix(key, "api.openshift.com") { + delete(hcp.Labels, key) + } + } for key, val := range hcluster.Labels { if strings.HasPrefix(key, "api.openshift.com") { hcp.Labels[key] = val diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_hcp_labels_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_hcp_labels_test.go new file mode 100644 index 00000000000..ae83d9c9e95 --- /dev/null +++ b/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) + } +} diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go new file mode 100644 index 00000000000..91a3b702107 --- /dev/null +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go @@ -0,0 +1,203 @@ +package hostedcluster + +import ( + "strings" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + hyperkarpenterv1 "github.com/openshift/hypershift/api/karpenter/v1" + hyperutil "github.com/openshift/hypershift/support/util" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +// hostedClusterMirroredAnnotations are the HostedCluster annotations consumed by reconciliation and mirrored to the HostedControlPlane. +// Keep this list in sync with reconcileHostedControlPlaneAnnotations. +var hostedClusterMirroredAnnotations = []string{ + hyperv1.DisablePKIReconciliationAnnotation, + hyperv1.OauthLoginURLOverrideAnnotation, + hyperv1.KonnectivityAgentImageAnnotation, + hyperv1.KonnectivityServerImageAnnotation, + hyperv1.ClusterAutoscalerImage, + hyperv1.IBMCloudKMSProviderImage, + hyperv1.AWSKMSProviderImage, + hyperv1.PortierisImageAnnotation, + hyperutil.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 input is in the HostedCluster AWS API. + "hypershift.openshift.io/aws-termination-handler-queue-url", + hyperv1.SwiftPodNetworkInstanceAnnotation, + hyperv1.EnableMetricsForwarding, +} + +// hostedClusterActionableAnnotationPrefixes are the HostedCluster annotation prefixes consumed by reconciliation and mirrored to the HostedControlPlane. +var hostedClusterActionableAnnotationPrefixes = []string{ + hyperv1.IdentityProviderOverridesAnnotationPrefix, + hyperv1.ResourceRequestOverrideAnnotationPrefix, +} + +var hostedClusterAdditionalActionableAnnotations = []string{ + hyperv1.RestartDateAnnotation, + hyperv1.ForceUpgradeToAnnotation, + hyperv1.AllowUnsupportedKubeVirtRHCOSVariantsAnnotation, + hyperv1.HCDestroyGracePeriodAnnotation, + hyperv1.PodSecurityAdmissionLabelOverrideAnnotation, + hyperv1.ClusterAPIManagerImage, + hyperv1.ClusterAPIProviderAWSImage, + hyperv1.ClusterAPIAzureProviderImage, + hyperv1.ClusterAPIGCPProviderImage, + hyperv1.ClusterAPIAgentProviderImage, + hyperv1.ClusterAPIKubeVirtProviderImage, + hyperv1.ClusterAPIPowerVSProviderImage, + hyperv1.ClusterAPIOpenStackProviderImage, + hyperv1.OpenStackResourceControllerImage, + hyperv1.SkipReleaseImageValidation, + hyperv1.SkipKASConflicSANValidation, + hyperv1.SkipControlPlaneNamespaceDeletionAnnotation, + hyperutil.HostedClustersScopeAnnotation, +} + +var hostedClusterActionableLabelPrefixes = []string{ + "api.openshift.com", +} + +func hostedClusterPrimaryPredicate(r client.Reader) predicate.Predicate { + return predicate.And( + hyperutil.PredicatesForHostedClusterAnnotationScoping(r), + predicate.Or( + predicate.GenerationChangedPredicate{}, + predicate.TypedFuncs[client.Object]{ + UpdateFunc: func(e event.TypedUpdateEvent[client.Object]) bool { + oldHC, ok := e.ObjectOld.(*hyperv1.HostedCluster) + if !ok || oldHC == nil { + return false + } + + newHC, ok := e.ObjectNew.(*hyperv1.HostedCluster) + if !ok || newHC == nil { + return false + } + + return hostedClusterDeletionTimestampChanged(oldHC, newHC) || + hostedClusterActionableAnnotationChanged(oldHC.GetAnnotations(), newHC.GetAnnotations()) || + hostedClusterActionableLabelChanged(oldHC.GetLabels(), newHC.GetLabels()) + }, + }, + ), + ) +} + +func hostedClusterActionableAnnotationChanged(oldAnnotations, newAnnotations map[string]string) bool { + for _, key := range hostedClusterMirroredAnnotations { + if annotationValueChanged(oldAnnotations, newAnnotations, key) { + return true + } + } + + for _, prefix := range hostedClusterActionableAnnotationPrefixes { + if annotationPrefixChanged(oldAnnotations, newAnnotations, prefix) { + return true + } + } + + for _, key := range hostedClusterAdditionalActionableAnnotations { + if annotationValueChanged(oldAnnotations, newAnnotations, key) { + return true + } + } + + return false +} + +func annotationValueChanged(oldAnnotations, newAnnotations map[string]string, key string) bool { + oldValue, oldHasValue := oldAnnotations[key] + newValue, newHasValue := newAnnotations[key] + + return oldHasValue != newHasValue || oldValue != newValue +} + +func annotationPrefixChanged(oldAnnotations, newAnnotations map[string]string, prefix string) bool { + oldPrefixedAnnotations := prefixedAnnotations(oldAnnotations, prefix) + newPrefixedAnnotations := prefixedAnnotations(newAnnotations, prefix) + + if len(oldPrefixedAnnotations) != len(newPrefixedAnnotations) { + return true + } + + for key, oldValue := range oldPrefixedAnnotations { + if newValue, ok := newPrefixedAnnotations[key]; !ok || newValue != oldValue { + return true + } + } + + return false +} + +func prefixedAnnotations(annotations map[string]string, prefix string) map[string]string { + prefixed := map[string]string{} + for key, value := range annotations { + if strings.HasPrefix(key, prefix) { + prefixed[key] = value + } + } + return prefixed +} + +func hostedClusterActionableLabelChanged(oldLabels, newLabels map[string]string) bool { + for _, prefix := range hostedClusterActionableLabelPrefixes { + if annotationPrefixChanged(oldLabels, newLabels, prefix) { + return true + } + } + + return false +} + +func hostedClusterDeletionTimestampChanged(oldHC, newHC *hyperv1.HostedCluster) bool { + oldDeletionTimestamp := oldHC.GetDeletionTimestamp() + newDeletionTimestamp := newHC.GetDeletionTimestamp() + + switch { + case oldDeletionTimestamp == nil && newDeletionTimestamp == nil: + return false + case oldDeletionTimestamp == nil || newDeletionTimestamp == nil: + return true + default: + return !oldDeletionTimestamp.Equal(newDeletionTimestamp) + } +} diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go new file mode 100644 index 00000000000..19ed4f12397 --- /dev/null +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go @@ -0,0 +1,380 @@ +package hostedcluster + +import ( + "testing" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + "github.com/openshift/hypershift/support/api" + hyperutil "github.com/openshift/hypershift/support/util" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/event" +) + +var platformProviderOverrideAnnotations = []string{ + hyperv1.ClusterAPIProviderAWSImage, + hyperv1.ClusterAPIAzureProviderImage, + hyperv1.ClusterAPIGCPProviderImage, + hyperv1.ClusterAPIAgentProviderImage, + hyperv1.ClusterAPIKubeVirtProviderImage, + hyperv1.ClusterAPIPowerVSProviderImage, + hyperv1.ClusterAPIOpenStackProviderImage, + hyperv1.OpenStackResourceControllerImage, +} + +func TestHostedClusterActionableAnnotationChanged_WhenAnnotationsChangeItShouldReturnExpectedResult(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + oldAnnotations map[string]string + newAnnotations map[string]string + expected bool + }{ + { + name: "When a mirrored annotation changes, it should return true", + oldAnnotations: map[string]string{ + hyperv1.CleanupCloudResourcesAnnotation: "false", + }, + newAnnotations: map[string]string{ + hyperv1.CleanupCloudResourcesAnnotation: "true", + }, + expected: true, + }, + { + name: "When a prefixed annotation changes, it should return true", + oldAnnotations: map[string]string{ + hyperv1.IdentityProviderOverridesAnnotationPrefix + "-example": "one", + }, + newAnnotations: map[string]string{ + hyperv1.IdentityProviderOverridesAnnotationPrefix + "-example": "two", + }, + expected: true, + }, + { + name: "When the restart annotation changes, it should return true", + oldAnnotations: map[string]string{ + hyperv1.RestartDateAnnotation: "2026-05-05T10:00:00Z", + }, + newAnnotations: map[string]string{ + hyperv1.RestartDateAnnotation: "2026-05-05T10:05:00Z", + }, + expected: true, + }, + { + name: "When a direct reconciliation annotation changes, it should return true", + oldAnnotations: map[string]string{ + hyperv1.ForceUpgradeToAnnotation: "quay.io/openshift-release-dev/ocp-release:4.19.0-x86_64", + }, + newAnnotations: map[string]string{ + hyperv1.ForceUpgradeToAnnotation: "quay.io/openshift-release-dev/ocp-release:4.19.1-x86_64", + }, + expected: true, + }, + { + name: "When the kubevirt escape hatch annotation changes, it should return true", + oldAnnotations: map[string]string{ + hyperv1.AllowUnsupportedKubeVirtRHCOSVariantsAnnotation: "false", + }, + newAnnotations: map[string]string{ + hyperv1.AllowUnsupportedKubeVirtRHCOSVariantsAnnotation: "true", + }, + expected: true, + }, + { + name: "When the destroy grace period annotation changes, it should return true", + oldAnnotations: map[string]string{ + hyperv1.HCDestroyGracePeriodAnnotation: "5m", + }, + newAnnotations: map[string]string{ + hyperv1.HCDestroyGracePeriodAnnotation: "10m", + }, + expected: true, + }, + { + name: "When the pod security override annotation changes, it should return true", + oldAnnotations: map[string]string{ + hyperv1.PodSecurityAdmissionLabelOverrideAnnotation: "privileged", + }, + newAnnotations: map[string]string{ + hyperv1.PodSecurityAdmissionLabelOverrideAnnotation: "restricted", + }, + expected: true, + }, + { + name: "When the cluster API manager image annotation changes, it should return true", + oldAnnotations: map[string]string{ + hyperv1.ClusterAPIManagerImage: "quay.io/example/capi:v1", + }, + newAnnotations: map[string]string{ + hyperv1.ClusterAPIManagerImage: "quay.io/example/capi:v2", + }, + expected: true, + }, + { + name: "When the skip release validation annotation changes, it should return true", + oldAnnotations: map[string]string{ + hyperv1.SkipReleaseImageValidation: "true", + }, + newAnnotations: map[string]string{}, + expected: true, + }, + { + name: "When the skip KAS conflict SAN validation annotation changes, it should return true", + oldAnnotations: map[string]string{ + hyperv1.SkipKASConflicSANValidation: "true", + }, + newAnnotations: map[string]string{}, + expected: true, + }, + { + name: "When the scope annotation changes, it should return true", + oldAnnotations: map[string]string{ + hyperutil.HostedClustersScopeAnnotation: "one", + }, + newAnnotations: map[string]string{ + hyperutil.HostedClustersScopeAnnotation: "two", + }, + expected: true, + }, + { + name: "When a non action annotation changes, it should return false", + oldAnnotations: map[string]string{ + "example.com/ignored": "old", + }, + newAnnotations: map[string]string{ + "example.com/ignored": "new", + }, + expected: false, + }, + { + name: "When actionable annotations do not change, it should return false", + oldAnnotations: map[string]string{ + hyperv1.CleanupCloudResourcesAnnotation: "true", + }, + newAnnotations: map[string]string{ + hyperv1.CleanupCloudResourcesAnnotation: "true", + }, + expected: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + if actual := hostedClusterActionableAnnotationChanged(tc.oldAnnotations, tc.newAnnotations); actual != tc.expected { + t.Fatalf("expected actionable annotation change to be %t, got %t", tc.expected, actual) + } + }) + } +} + +func TestHostedClusterActionableAnnotationChanged_WhenAPlatformProviderOverrideChangesItShouldReturnTrue(t *testing.T) { + t.Parallel() + + for _, annotation := range platformProviderOverrideAnnotations { + t.Run("When "+annotation+" changes, it should return true", func(t *testing.T) { + t.Parallel() + + if actual := hostedClusterActionableAnnotationChanged( + map[string]string{annotation: "old"}, + map[string]string{annotation: "new"}, + ); !actual { + t.Fatalf("expected annotation %s to be actionable", annotation) + } + }) + } +} + +func TestHostedClusterPrimaryPredicate_WhenHostedClusterUpdatesItShouldFilterMeaningfulChanges(t *testing.T) { + t.Setenv(hyperutil.EnableHostedClustersAnnotationScopingEnv, "") + t.Setenv(hyperutil.HostedClustersScopeAnnotationEnv, "") + + predicate := hostedClusterPrimaryPredicate(fake.NewClientBuilder().WithScheme(api.Scheme).Build()) + + testCases := []struct { + name string + oldHC *hyperv1.HostedCluster + newHC *hyperv1.HostedCluster + expected bool + }{ + { + name: "When the generation changes, it should allow the update", + oldHC: newHostedClusterForPredicateTests(1, nil), + newHC: newHostedClusterForPredicateTests(2, nil), + expected: true, + }, + { + name: "When only status changes, it should skip the update", + oldHC: newHostedClusterForPredicateTests(1, nil), + newHC: func() *hyperv1.HostedCluster { + hc := newHostedClusterForPredicateTests(1, nil) + hc.Status.Conditions = []metav1.Condition{{ + Type: string(hyperv1.ReconciliationSucceeded), + Status: metav1.ConditionTrue, + }} + return hc + }(), + expected: false, + }, + { + name: "When a mirrored annotation changes, it should allow the update", + oldHC: newHostedClusterForPredicateTests(1, map[string]string{ + hyperv1.CleanupCloudResourcesAnnotation: "false", + }), + newHC: newHostedClusterForPredicateTests(1, map[string]string{ + hyperv1.CleanupCloudResourcesAnnotation: "true", + }), + expected: true, + }, + { + name: "When a prefixed annotation changes, it should allow the update", + oldHC: newHostedClusterForPredicateTests(1, map[string]string{ + hyperv1.ResourceRequestOverrideAnnotationPrefix + "-kas": "old", + }), + newHC: newHostedClusterForPredicateTests(1, map[string]string{ + hyperv1.ResourceRequestOverrideAnnotationPrefix + "-kas": "new", + }), + expected: true, + }, + { + name: "When a direct reconciliation annotation changes, it should allow the update", + oldHC: newHostedClusterForPredicateTests(1, map[string]string{ + hyperv1.ForceUpgradeToAnnotation: "quay.io/openshift-release-dev/ocp-release:4.19.0-x86_64", + }), + newHC: newHostedClusterForPredicateTests(1, map[string]string{ + hyperv1.ForceUpgradeToAnnotation: "quay.io/openshift-release-dev/ocp-release:4.19.1-x86_64", + }), + expected: true, + }, + { + name: "When a platform provider override annotation changes, it should allow the update", + oldHC: newHostedClusterForPredicateTests(1, map[string]string{ + hyperv1.ClusterAPIProviderAWSImage: "quay.io/example/aws:v1", + }), + newHC: newHostedClusterForPredicateTests(1, map[string]string{ + hyperv1.ClusterAPIProviderAWSImage: "quay.io/example/aws:v2", + }), + expected: true, + }, + { + name: "When the KAS SAN validation skip annotation changes, it should allow the update", + oldHC: newHostedClusterForPredicateTests(1, map[string]string{ + hyperv1.SkipKASConflicSANValidation: "true", + }), + newHC: newHostedClusterForPredicateTests(1, map[string]string{}), + expected: true, + }, + { + name: "When the scope annotation changes, it should allow the update", + oldHC: newHostedClusterForPredicateTests(1, map[string]string{ + hyperutil.HostedClustersScopeAnnotation: "one", + }), + newHC: newHostedClusterForPredicateTests(1, map[string]string{ + hyperutil.HostedClustersScopeAnnotation: "two", + }), + expected: true, + }, + { + name: "When the deletion timestamp changes, it should allow the update", + oldHC: newHostedClusterForPredicateTests(1, nil), + newHC: func() *hyperv1.HostedCluster { + hc := newHostedClusterForPredicateTests(1, nil) + hc.DeletionTimestamp = ptrToTime(metav1.Now()) + return hc + }(), + expected: true, + }, + { + name: "When an actionable label changes, it should allow the update", + oldHC: func() *hyperv1.HostedCluster { + hc := newHostedClusterForPredicateTests(1, nil) + hc.Labels = map[string]string{"api.openshift.com/id": "old"} + return hc + }(), + newHC: func() *hyperv1.HostedCluster { + hc := newHostedClusterForPredicateTests(1, nil) + hc.Labels = map[string]string{"api.openshift.com/id": "new"} + return hc + }(), + expected: true, + }, + { + name: "When an actionable label is removed, it should allow the update", + oldHC: func() *hyperv1.HostedCluster { + hc := newHostedClusterForPredicateTests(1, nil) + hc.Labels = map[string]string{"api.openshift.com/id": "old"} + return hc + }(), + newHC: func() *hyperv1.HostedCluster { + hc := newHostedClusterForPredicateTests(1, nil) + hc.Labels = map[string]string{} + return hc + }(), + expected: true, + }, + { + name: "When a non action annotation changes, it should skip the update", + oldHC: newHostedClusterForPredicateTests(1, map[string]string{ + "example.com/ignored": "old", + }), + newHC: newHostedClusterForPredicateTests(1, map[string]string{ + "example.com/ignored": "new", + }), + expected: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + actual := predicate.Update(event.UpdateEvent{ + ObjectOld: tc.oldHC, + ObjectNew: tc.newHC, + }) + if actual != tc.expected { + t.Fatalf("expected predicate result %t, got %t", tc.expected, actual) + } + }) + } +} + +func TestHostedClusterPrimaryPredicate_WhenAHostedClusterBecomesInScopeItShouldAllowTheUpdate(t *testing.T) { + t.Setenv(hyperutil.EnableHostedClustersAnnotationScopingEnv, "true") + t.Setenv(hyperutil.HostedClustersScopeAnnotationEnv, "team-a") + + predicate := hostedClusterPrimaryPredicate(fake.NewClientBuilder().WithScheme(api.Scheme).Build()) + + oldHC := newHostedClusterForPredicateTests(1, map[string]string{ + hyperutil.HostedClustersScopeAnnotation: "team-b", + }) + newHC := newHostedClusterForPredicateTests(1, map[string]string{ + hyperutil.HostedClustersScopeAnnotation: "team-a", + }) + + if !predicate.Update(event.UpdateEvent{ + ObjectOld: oldHC, + ObjectNew: newHC, + }) { + t.Fatal("expected scope transition into the configured scope to enqueue a reconcile") + } +} + +func newHostedClusterForPredicateTests(generation int64, annotations map[string]string) *hyperv1.HostedCluster { + return &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "clusters", + Generation: generation, + Annotations: annotations, + }, + } +} + +func ptrToTime(value metav1.Time) *metav1.Time { + return &value +} diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_reconciliation_condition_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_reconciliation_condition_test.go new file mode 100644 index 00000000000..27b3e2f1f71 --- /dev/null +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_reconciliation_condition_test.go @@ -0,0 +1,227 @@ +package hostedcluster + +import ( + "context" + "errors" + "testing" + "time" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + "github.com/openshift/hypershift/support/api" + + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + ctrl "sigs.k8s.io/controller-runtime" + crclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/go-logr/logr" +) + +func TestReconcile_WhenReconciliationFinishesItShouldStampObservedGenerationOnTheCondition(t *testing.T) { + t.Parallel() + + reconcilerNow := metav1.Time{Time: time.Now().Round(time.Second)} + const generation int64 = 7 + + testCases := []struct { + name string + reconcileError error + expectedStatus metav1.ConditionStatus + expectedReason string + expectedMsg string + }{ + { + name: "When reconciliation succeeds, it should stamp the current generation", + expectedStatus: metav1.ConditionTrue, + expectedReason: "ReconciliatonSucceeded", + expectedMsg: "Reconciliation completed successfully", + }, + { + name: "When reconciliation fails, it should stamp the failing generation", + reconcileError: errors.New("things went sideways"), + expectedStatus: metav1.ConditionFalse, + expectedReason: "ReconciliationError", + expectedMsg: "things went sideways", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + hcluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "clusters", + Generation: generation, + }, + } + + c := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(hcluster).WithStatusSubresource(hcluster).Build() + r := &HostedClusterReconciler{ + Client: c, + CertRotationScale: 24 * time.Hour, + overwriteReconcile: func(ctx context.Context, req ctrl.Request, log logr.Logger, hcluster *hyperv1.HostedCluster) (ctrl.Result, error) { + return ctrl.Result{}, tc.reconcileError + }, + now: func() metav1.Time { return reconcilerNow }, + } + + _, _ = r.Reconcile(t.Context(), ctrl.Request{NamespacedName: crclient.ObjectKeyFromObject(hcluster)}) + + if err := c.Get(t.Context(), crclient.ObjectKeyFromObject(hcluster), hcluster); err != nil { + t.Fatalf("failed to get hostedcluster after reconciliation: %v", err) + } + + condition := meta.FindStatusCondition(hcluster.Status.Conditions, string(hyperv1.ReconciliationSucceeded)) + if condition == nil { + t.Fatalf("expected %s condition to be set", hyperv1.ReconciliationSucceeded) + } + if condition.ObservedGeneration != generation { + t.Fatalf("expected observed generation %d, got %d", generation, condition.ObservedGeneration) + } + if condition.Status != tc.expectedStatus { + t.Fatalf("expected condition status %s, got %s", tc.expectedStatus, condition.Status) + } + if condition.Reason != tc.expectedReason { + t.Fatalf("expected condition reason %s, got %s", tc.expectedReason, condition.Reason) + } + if condition.Message != tc.expectedMsg { + t.Fatalf("expected condition message %q, got %q", tc.expectedMsg, condition.Message) + } + }) + } +} + +func TestReconcile_WhenQueuedRequestsAreStaleItShouldReadTheLatestHostedClusterGeneration(t *testing.T) { + t.Parallel() + + const latestGeneration int64 = 3 + reconcilerNow := metav1.Time{Time: time.Now().Round(time.Second)} + + hcluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "clusters", + Generation: 1, + }, + } + + c := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(hcluster).WithStatusSubresource(hcluster).Build() + + storedHostedCluster := &hyperv1.HostedCluster{} + if err := c.Get(t.Context(), crclient.ObjectKeyFromObject(hcluster), storedHostedCluster); err != nil { + t.Fatalf("failed to get hostedcluster before updating generation: %v", err) + } + storedHostedCluster.Generation = latestGeneration + if err := c.Update(t.Context(), storedHostedCluster); err != nil { + t.Fatalf("failed to update hostedcluster generation before reconcile: %v", err) + } + + var reconciledGenerations []int64 + r := &HostedClusterReconciler{ + Client: c, + CertRotationScale: 24 * time.Hour, + overwriteReconcile: func(ctx context.Context, req ctrl.Request, log logr.Logger, hcluster *hyperv1.HostedCluster) (ctrl.Result, error) { + reconciledGenerations = append(reconciledGenerations, hcluster.Generation) + return ctrl.Result{}, nil + }, + now: func() metav1.Time { return reconcilerNow }, + } + + request := ctrl.Request{NamespacedName: crclient.ObjectKeyFromObject(hcluster)} + for i := 0; i < 2; i++ { + if _, err := r.Reconcile(t.Context(), request); err != nil { + t.Fatalf("reconcile %d failed: %v", i, err) + } + } + + if len(reconciledGenerations) != 2 { + t.Fatalf("expected 2 reconciliations, got %d", len(reconciledGenerations)) + } + for _, generation := range reconciledGenerations { + if generation != latestGeneration { + t.Fatalf("expected all reconciliations to read generation %d, got %v", latestGeneration, reconciledGenerations) + } + } + + if err := c.Get(t.Context(), crclient.ObjectKeyFromObject(hcluster), storedHostedCluster); err != nil { + t.Fatalf("failed to get hostedcluster after reconciliation: %v", err) + } + condition := meta.FindStatusCondition(storedHostedCluster.Status.Conditions, string(hyperv1.ReconciliationSucceeded)) + if condition == nil { + t.Fatalf("expected %s condition to be set", hyperv1.ReconciliationSucceeded) + } + if condition.ObservedGeneration != latestGeneration { + t.Fatalf("expected observed generation %d, got %d", latestGeneration, condition.ObservedGeneration) + } +} + +func TestReconcile_WhenQueuedRequestsAreStaleAndActionableMetadataChangesItShouldReadTheLatestHostedClusterState(t *testing.T) { + t.Parallel() + + reconcilerNow := metav1.Time{Time: time.Now().Round(time.Second)} + const latestProviderImage = "quay.io/example/aws:v2" + + hcluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "clusters", + Generation: 7, + Annotations: map[string]string{ + hyperv1.ClusterAPIProviderAWSImage: "quay.io/example/aws:v1", + }, + }, + } + + c := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(hcluster).WithStatusSubresource(hcluster).Build() + + storedHostedCluster := &hyperv1.HostedCluster{} + if err := c.Get(t.Context(), crclient.ObjectKeyFromObject(hcluster), storedHostedCluster); err != nil { + t.Fatalf("failed to get hostedcluster before updating annotations: %v", err) + } + storedHostedCluster.Annotations[hyperv1.ClusterAPIProviderAWSImage] = latestProviderImage + if err := c.Update(t.Context(), storedHostedCluster); err != nil { + t.Fatalf("failed to update hostedcluster annotations before reconcile: %v", err) + } + + var reconciledProviderImages []string + r := &HostedClusterReconciler{ + Client: c, + CertRotationScale: 24 * time.Hour, + overwriteReconcile: func(ctx context.Context, req ctrl.Request, log logr.Logger, hcluster *hyperv1.HostedCluster) (ctrl.Result, error) { + reconciledProviderImages = append(reconciledProviderImages, hcluster.Annotations[hyperv1.ClusterAPIProviderAWSImage]) + return ctrl.Result{}, nil + }, + now: func() metav1.Time { return reconcilerNow }, + } + + request := ctrl.Request{NamespacedName: crclient.ObjectKeyFromObject(hcluster)} + for i := 0; i < 2; i++ { + if _, err := r.Reconcile(t.Context(), request); err != nil { + t.Fatalf("reconcile %d failed: %v", i, err) + } + } + + if len(reconciledProviderImages) != 2 { + t.Fatalf("expected 2 reconciliations, got %d", len(reconciledProviderImages)) + } + for _, providerImage := range reconciledProviderImages { + if providerImage != latestProviderImage { + t.Fatalf("expected all reconciliations to read provider image %q, got %v", latestProviderImage, reconciledProviderImages) + } + } + + if err := c.Get(t.Context(), crclient.ObjectKeyFromObject(hcluster), storedHostedCluster); err != nil { + t.Fatalf("failed to get hostedcluster after reconciliation: %v", err) + } + condition := meta.FindStatusCondition(storedHostedCluster.Status.Conditions, string(hyperv1.ReconciliationSucceeded)) + if condition == nil { + t.Fatalf("expected %s condition to be set", hyperv1.ReconciliationSucceeded) + } + if condition.ObservedGeneration != hcluster.Generation { + t.Fatalf("expected observed generation %d, got %d", hcluster.Generation, condition.ObservedGeneration) + } +} From cc363d5c3064f8448810fce1e959898cf62e7581 Mon Sep 17 00:00:00 2001 From: Alessandro Affinito Date: Tue, 5 May 2026 16:27:12 +0200 Subject: [PATCH 02/11] fix(hostedcluster): revalidate release image after skip toggles Track when release image validation was skipped so same-generation annotation changes can invalidate a cached True ValidReleaseImage condition. Add a focused regression test matrix covering skip annotation add and remove transitions without a generation bump. Signed-off-by: Alessandro Affinito Commit-Message-Assisted-by: Claude (via Claude Code) Co-authored-by: Cursor --- .../hostedcluster/hostedcluster_controller.go | 33 ++++- ...edcluster_release_image_validation_test.go | 126 ++++++++++++++++++ 2 files changed, 155 insertions(+), 4 deletions(-) create mode 100644 hypershift-operator/controllers/hostedcluster/hostedcluster_release_image_validation_test.go diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index 162bbe37d18..0d028127eb8 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -146,6 +146,7 @@ const ( previouslySyncedRestartDateAnnotation = "hypershift.openshift.io/previous-restart-date" kasServingCertHashAnnotation = "hypershift.openshift.io/kas-serving-cert-hash" referencedResourceAnnotationPrefix = "referenced-resource.hypershift.openshift.io/" + releaseImageValidationSkippedReason = "ReleaseImageValidationSkipped" ) var ( @@ -1187,13 +1188,19 @@ 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, } + skipReleaseImageValidation := hasSkipReleaseImageValidationAnnotation(hcluster) err := r.validateReleaseImage(ctx, hcluster, releaseProvider) - if err != nil { + switch { + case skipReleaseImageValidation: + condition.Status = metav1.ConditionTrue + condition.Message = "Release image validation is skipped by annotation" + condition.Reason = releaseImageValidationSkippedReason + case err != nil: condition.Status = metav1.ConditionFalse condition.Message = err.Error() @@ -1202,7 +1209,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 @@ -3692,7 +3699,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) @@ -3727,6 +3734,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 != releaseImageValidationSkippedReason + } + + return condition.Reason == 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) { diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_release_image_validation_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_release_image_validation_test.go new file mode 100644 index 00000000000..2ae15ec4939 --- /dev/null +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_release_image_validation_test.go @@ -0,0 +1,126 @@ +package hostedcluster + +import ( + "testing" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestShouldValidateReleaseImage_WhenReleaseImageValidationInputsChangeItShouldReturnExpectedResult(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + hcluster *hyperv1.HostedCluster + condition *metav1.Condition + expected bool + }{ + { + name: "When there is no existing condition, it should validate", + hcluster: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{Generation: 3}, + }, + expected: true, + }, + { + name: "When the existing condition observed generation is stale, it should validate", + hcluster: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{Generation: 3}, + }, + condition: &metav1.Condition{ + Type: string(hyperv1.ValidReleaseImage), + Status: metav1.ConditionTrue, + ObservedGeneration: 2, + Reason: hyperv1.AsExpectedReason, + }, + expected: true, + }, + { + name: "When the existing condition is not true, it should validate", + hcluster: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{Generation: 3}, + }, + condition: &metav1.Condition{ + Type: string(hyperv1.ValidReleaseImage), + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: hyperv1.InvalidImageReason, + }, + expected: true, + }, + { + name: "When the skip annotation remains present with a skipped condition, it should not validate", + hcluster: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 3, + Annotations: map[string]string{ + hyperv1.SkipReleaseImageValidation: "true", + }, + }, + }, + condition: &metav1.Condition{ + Type: string(hyperv1.ValidReleaseImage), + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + Reason: releaseImageValidationSkippedReason, + }, + expected: false, + }, + { + name: "When the skip annotation is removed after a skipped condition, it should validate", + hcluster: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{Generation: 3}, + }, + condition: &metav1.Condition{ + Type: string(hyperv1.ValidReleaseImage), + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + Reason: releaseImageValidationSkippedReason, + }, + expected: true, + }, + { + name: "When the skip annotation is added after a valid condition, it should validate", + hcluster: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 3, + Annotations: map[string]string{ + hyperv1.SkipReleaseImageValidation: "true", + }, + }, + }, + condition: &metav1.Condition{ + Type: string(hyperv1.ValidReleaseImage), + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + Reason: hyperv1.AsExpectedReason, + }, + expected: true, + }, + { + name: "When the generation and validation inputs are unchanged, it should not validate", + hcluster: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{Generation: 3}, + }, + condition: &metav1.Condition{ + Type: string(hyperv1.ValidReleaseImage), + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + Reason: hyperv1.AsExpectedReason, + }, + expected: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + if actual := shouldValidateReleaseImage(tc.hcluster, tc.condition); actual != tc.expected { + t.Fatalf("expected shouldValidateReleaseImage to return %t, got %t", tc.expected, actual) + } + }) + } +} From 553451fba7537544a63639ee3bc4599490c40661 Mon Sep 17 00:00:00 2001 From: Alessandro Affinito Date: Tue, 5 May 2026 17:27:18 +0200 Subject: [PATCH 03/11] fix(hostedcluster): align predicate constants after rebase Update the new HostedCluster predicate code to use the k8sutil-scoped annotation constants introduced on current main so the branch verifies cleanly after rebasing. Co-authored-by: Cursor --- .../hostedcluster/hostedcluster_predicates.go | 5 +++-- .../hostedcluster_predicates_test.go | 22 +++++++++---------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go index 91a3b702107..6c35c97d2ae 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go @@ -5,6 +5,7 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" hyperkarpenterv1 "github.com/openshift/hypershift/api/karpenter/v1" + "github.com/openshift/hypershift/support/k8sutil" hyperutil "github.com/openshift/hypershift/support/util" "sigs.k8s.io/controller-runtime/pkg/client" @@ -23,7 +24,7 @@ var hostedClusterMirroredAnnotations = []string{ hyperv1.IBMCloudKMSProviderImage, hyperv1.AWSKMSProviderImage, hyperv1.PortierisImageAnnotation, - hyperutil.DebugDeploymentsAnnotation, + k8sutil.DebugDeploymentsAnnotation, hyperv1.DisableProfilingAnnotation, hyperv1.PrivateIngressControllerAnnotation, hyperv1.IngressControllerLoadBalancerScope, @@ -89,7 +90,7 @@ var hostedClusterAdditionalActionableAnnotations = []string{ hyperv1.SkipReleaseImageValidation, hyperv1.SkipKASConflicSANValidation, hyperv1.SkipControlPlaneNamespaceDeletionAnnotation, - hyperutil.HostedClustersScopeAnnotation, + k8sutil.HostedClustersScopeAnnotation, } var hostedClusterActionableLabelPrefixes = []string{ diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go index 19ed4f12397..55050393ccf 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go @@ -5,7 +5,7 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/support/api" - hyperutil "github.com/openshift/hypershift/support/util" + "github.com/openshift/hypershift/support/k8sutil" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -132,10 +132,10 @@ func TestHostedClusterActionableAnnotationChanged_WhenAnnotationsChangeItShouldR { name: "When the scope annotation changes, it should return true", oldAnnotations: map[string]string{ - hyperutil.HostedClustersScopeAnnotation: "one", + k8sutil.HostedClustersScopeAnnotation: "one", }, newAnnotations: map[string]string{ - hyperutil.HostedClustersScopeAnnotation: "two", + k8sutil.HostedClustersScopeAnnotation: "two", }, expected: true, }, @@ -190,8 +190,8 @@ func TestHostedClusterActionableAnnotationChanged_WhenAPlatformProviderOverrideC } func TestHostedClusterPrimaryPredicate_WhenHostedClusterUpdatesItShouldFilterMeaningfulChanges(t *testing.T) { - t.Setenv(hyperutil.EnableHostedClustersAnnotationScopingEnv, "") - t.Setenv(hyperutil.HostedClustersScopeAnnotationEnv, "") + t.Setenv(k8sutil.EnableHostedClustersAnnotationScopingEnv, "") + t.Setenv(k8sutil.HostedClustersScopeAnnotationEnv, "") predicate := hostedClusterPrimaryPredicate(fake.NewClientBuilder().WithScheme(api.Scheme).Build()) @@ -271,10 +271,10 @@ func TestHostedClusterPrimaryPredicate_WhenHostedClusterUpdatesItShouldFilterMea { name: "When the scope annotation changes, it should allow the update", oldHC: newHostedClusterForPredicateTests(1, map[string]string{ - hyperutil.HostedClustersScopeAnnotation: "one", + k8sutil.HostedClustersScopeAnnotation: "one", }), newHC: newHostedClusterForPredicateTests(1, map[string]string{ - hyperutil.HostedClustersScopeAnnotation: "two", + k8sutil.HostedClustersScopeAnnotation: "two", }), expected: true, }, @@ -344,16 +344,16 @@ func TestHostedClusterPrimaryPredicate_WhenHostedClusterUpdatesItShouldFilterMea } func TestHostedClusterPrimaryPredicate_WhenAHostedClusterBecomesInScopeItShouldAllowTheUpdate(t *testing.T) { - t.Setenv(hyperutil.EnableHostedClustersAnnotationScopingEnv, "true") - t.Setenv(hyperutil.HostedClustersScopeAnnotationEnv, "team-a") + t.Setenv(k8sutil.EnableHostedClustersAnnotationScopingEnv, "true") + t.Setenv(k8sutil.HostedClustersScopeAnnotationEnv, "team-a") predicate := hostedClusterPrimaryPredicate(fake.NewClientBuilder().WithScheme(api.Scheme).Build()) oldHC := newHostedClusterForPredicateTests(1, map[string]string{ - hyperutil.HostedClustersScopeAnnotation: "team-b", + k8sutil.HostedClustersScopeAnnotation: "team-b", }) newHC := newHostedClusterForPredicateTests(1, map[string]string{ - hyperutil.HostedClustersScopeAnnotation: "team-a", + k8sutil.HostedClustersScopeAnnotation: "team-a", }) if !predicate.Update(event.UpdateEvent{ From 1280cf1af4a9dddc522b6d248d3fa10aab5aa4c9 Mon Sep 17 00:00:00 2001 From: Alessandro Affinito Date: Wed, 6 May 2026 09:45:40 +0200 Subject: [PATCH 04/11] refactor(hostedcluster): reuse actionable label prefixes in HCP sync Keep HostedControlPlane label mirroring aligned with the shared actionable label prefix list so the watch predicate and reconciliation logic stay in sync. Co-authored-by: Cursor --- .../hostedcluster/hostedcluster_controller.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index 0d028127eb8..2cf0980e90b 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -2397,13 +2397,19 @@ 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 { - if strings.HasPrefix(key, "api.openshift.com") { - delete(hcp.Labels, key) + 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 + } } } From 037e3469f3afc4e63fe9e2aff4e3323e0f1b01cf Mon Sep 17 00:00:00 2001 From: Alessandro Affinito Date: Wed, 6 May 2026 10:29:42 +0200 Subject: [PATCH 05/11] test(hostedcluster): use static names for predicate subtests Replace the dynamic platform override subtest titles with explicit static names so the HostedCluster predicate tests satisfy deterministic naming checks without changing the existing assertion style used in this package. Signed-off-by: Alessandro Affinito Commit-Message-Assisted-by: Claude (via Claude Code) Co-authored-by: Cursor --- .../hostedcluster_predicates_test.go | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go index 55050393ccf..211264340c9 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go @@ -13,15 +13,18 @@ import ( "sigs.k8s.io/controller-runtime/pkg/event" ) -var platformProviderOverrideAnnotations = []string{ - hyperv1.ClusterAPIProviderAWSImage, - hyperv1.ClusterAPIAzureProviderImage, - hyperv1.ClusterAPIGCPProviderImage, - hyperv1.ClusterAPIAgentProviderImage, - hyperv1.ClusterAPIKubeVirtProviderImage, - hyperv1.ClusterAPIPowerVSProviderImage, - hyperv1.ClusterAPIOpenStackProviderImage, - hyperv1.OpenStackResourceControllerImage, +var platformProviderOverrideAnnotations = []struct { + name string + annotation string +}{ + {name: "When the AWS provider image override changes, it should return true", annotation: hyperv1.ClusterAPIProviderAWSImage}, + {name: "When the Azure provider image override changes, it should return true", annotation: hyperv1.ClusterAPIAzureProviderImage}, + {name: "When the GCP provider image override changes, it should return true", annotation: hyperv1.ClusterAPIGCPProviderImage}, + {name: "When the agent provider image override changes, it should return true", annotation: hyperv1.ClusterAPIAgentProviderImage}, + {name: "When the KubeVirt provider image override changes, it should return true", annotation: hyperv1.ClusterAPIKubeVirtProviderImage}, + {name: "When the PowerVS provider image override changes, it should return true", annotation: hyperv1.ClusterAPIPowerVSProviderImage}, + {name: "When the OpenStack provider image override changes, it should return true", annotation: hyperv1.ClusterAPIOpenStackProviderImage}, + {name: "When the OpenStack resource controller image override changes, it should return true", annotation: hyperv1.OpenStackResourceControllerImage}, } func TestHostedClusterActionableAnnotationChanged_WhenAnnotationsChangeItShouldReturnExpectedResult(t *testing.T) { @@ -175,15 +178,15 @@ func TestHostedClusterActionableAnnotationChanged_WhenAnnotationsChangeItShouldR func TestHostedClusterActionableAnnotationChanged_WhenAPlatformProviderOverrideChangesItShouldReturnTrue(t *testing.T) { t.Parallel() - for _, annotation := range platformProviderOverrideAnnotations { - t.Run("When "+annotation+" changes, it should return true", func(t *testing.T) { + for _, tc := range platformProviderOverrideAnnotations { + t.Run(tc.name, func(t *testing.T) { t.Parallel() if actual := hostedClusterActionableAnnotationChanged( - map[string]string{annotation: "old"}, - map[string]string{annotation: "new"}, + map[string]string{tc.annotation: "old"}, + map[string]string{tc.annotation: "new"}, ); !actual { - t.Fatalf("expected annotation %s to be actionable", annotation) + t.Fatalf("expected annotation %s to be actionable", tc.annotation) } }) } From 98cfe794e457577626752d17ec74ea5dec5ef481 Mon Sep 17 00:00:00 2001 From: Alessandro Affinito Date: Fri, 22 May 2026 11:31:23 +0200 Subject: [PATCH 06/11] refactor(hostedcluster): rename annotation-specific helpers to map-generic names annotationPrefixChanged and prefixedAnnotations are used for both annotations and labels. Rename to prefixedKeyChanged and prefixedEntries with generic parameter names to avoid reader confusion when the functions are called with label maps. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../hostedcluster/hostedcluster_predicates.go | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go index 6c35c97d2ae..91461dfdf3e 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go @@ -131,7 +131,7 @@ func hostedClusterActionableAnnotationChanged(oldAnnotations, newAnnotations map } for _, prefix := range hostedClusterActionableAnnotationPrefixes { - if annotationPrefixChanged(oldAnnotations, newAnnotations, prefix) { + if prefixedKeyChanged(oldAnnotations, newAnnotations, prefix) { return true } } @@ -152,16 +152,16 @@ func annotationValueChanged(oldAnnotations, newAnnotations map[string]string, ke return oldHasValue != newHasValue || oldValue != newValue } -func annotationPrefixChanged(oldAnnotations, newAnnotations map[string]string, prefix string) bool { - oldPrefixedAnnotations := prefixedAnnotations(oldAnnotations, prefix) - newPrefixedAnnotations := prefixedAnnotations(newAnnotations, prefix) +func prefixedKeyChanged(old, new map[string]string, prefix string) bool { + oldEntries := prefixedEntries(old, prefix) + newEntries := prefixedEntries(new, prefix) - if len(oldPrefixedAnnotations) != len(newPrefixedAnnotations) { + if len(oldEntries) != len(newEntries) { return true } - for key, oldValue := range oldPrefixedAnnotations { - if newValue, ok := newPrefixedAnnotations[key]; !ok || newValue != oldValue { + for key, oldValue := range oldEntries { + if newValue, ok := newEntries[key]; !ok || newValue != oldValue { return true } } @@ -169,19 +169,19 @@ func annotationPrefixChanged(oldAnnotations, newAnnotations map[string]string, p return false } -func prefixedAnnotations(annotations map[string]string, prefix string) map[string]string { - prefixed := map[string]string{} - for key, value := range annotations { +func prefixedEntries(m map[string]string, prefix string) map[string]string { + result := map[string]string{} + for key, value := range m { if strings.HasPrefix(key, prefix) { - prefixed[key] = value + result[key] = value } } - return prefixed + return result } func hostedClusterActionableLabelChanged(oldLabels, newLabels map[string]string) bool { for _, prefix := range hostedClusterActionableLabelPrefixes { - if annotationPrefixChanged(oldLabels, newLabels, prefix) { + if prefixedKeyChanged(oldLabels, newLabels, prefix) { return true } } From 258cb70d0f51be25eb897789b3a0dacbed2c443b Mon Sep 17 00:00:00 2001 From: Alessandro Affinito Date: Fri, 22 May 2026 11:32:12 +0200 Subject: [PATCH 07/11] fix(hostedcluster): skip validateReleaseImage call when skip annotation is present Guard the validateReleaseImage call so it is not invoked when the skip annotation is set. Previously the call ran but its result was discarded by the switch branch, relying on an internal short-circuit. Making the guard explicit avoids unnecessary work and makes intent clearer. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../controllers/hostedcluster/hostedcluster_controller.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index 2cf0980e90b..c9f6dfc8dd1 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -1194,7 +1194,10 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques ObservedGeneration: hcluster.Generation, } skipReleaseImageValidation := hasSkipReleaseImageValidationAnnotation(hcluster) - err := r.validateReleaseImage(ctx, hcluster, releaseProvider) + var err error + if !skipReleaseImageValidation { + err = r.validateReleaseImage(ctx, hcluster, releaseProvider) + } switch { case skipReleaseImageValidation: condition.Status = metav1.ConditionTrue From 705cfef3475fac55a79970de7cfb13332669dc8c Mon Sep 17 00:00:00 2001 From: Alessandro Affinito Date: Fri, 22 May 2026 11:33:44 +0200 Subject: [PATCH 08/11] test(hostedcluster): use ptr.To and avoid shadowing predicate package Replace the local ptrToTime helper with ptr.To from k8s.io/utils/ptr, which is already a dependency in this package. Rename the predicate local variable to pred so it does not shadow the controller-runtime predicate package import. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../hostedcluster/hostedcluster_predicates_test.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go index 211264340c9..3f29a2a9e2e 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go @@ -8,6 +8,7 @@ import ( "github.com/openshift/hypershift/support/k8sutil" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/event" @@ -196,7 +197,7 @@ func TestHostedClusterPrimaryPredicate_WhenHostedClusterUpdatesItShouldFilterMea t.Setenv(k8sutil.EnableHostedClustersAnnotationScopingEnv, "") t.Setenv(k8sutil.HostedClustersScopeAnnotationEnv, "") - predicate := hostedClusterPrimaryPredicate(fake.NewClientBuilder().WithScheme(api.Scheme).Build()) + pred := hostedClusterPrimaryPredicate(fake.NewClientBuilder().WithScheme(api.Scheme).Build()) testCases := []struct { name string @@ -286,7 +287,7 @@ func TestHostedClusterPrimaryPredicate_WhenHostedClusterUpdatesItShouldFilterMea oldHC: newHostedClusterForPredicateTests(1, nil), newHC: func() *hyperv1.HostedCluster { hc := newHostedClusterForPredicateTests(1, nil) - hc.DeletionTimestamp = ptrToTime(metav1.Now()) + hc.DeletionTimestamp = ptr.To(metav1.Now()) return hc }(), expected: true, @@ -335,7 +336,7 @@ func TestHostedClusterPrimaryPredicate_WhenHostedClusterUpdatesItShouldFilterMea t.Run(tc.name, func(t *testing.T) { t.Parallel() - actual := predicate.Update(event.UpdateEvent{ + actual := pred.Update(event.UpdateEvent{ ObjectOld: tc.oldHC, ObjectNew: tc.newHC, }) @@ -350,7 +351,7 @@ func TestHostedClusterPrimaryPredicate_WhenAHostedClusterBecomesInScopeItShouldA t.Setenv(k8sutil.EnableHostedClustersAnnotationScopingEnv, "true") t.Setenv(k8sutil.HostedClustersScopeAnnotationEnv, "team-a") - predicate := hostedClusterPrimaryPredicate(fake.NewClientBuilder().WithScheme(api.Scheme).Build()) + pred := hostedClusterPrimaryPredicate(fake.NewClientBuilder().WithScheme(api.Scheme).Build()) oldHC := newHostedClusterForPredicateTests(1, map[string]string{ k8sutil.HostedClustersScopeAnnotation: "team-b", @@ -359,7 +360,7 @@ func TestHostedClusterPrimaryPredicate_WhenAHostedClusterBecomesInScopeItShouldA k8sutil.HostedClustersScopeAnnotation: "team-a", }) - if !predicate.Update(event.UpdateEvent{ + if !pred.Update(event.UpdateEvent{ ObjectOld: oldHC, ObjectNew: newHC, }) { @@ -378,6 +379,3 @@ func newHostedClusterForPredicateTests(generation int64, annotations map[string] } } -func ptrToTime(value metav1.Time) *metav1.Time { - return &value -} From 18d2ac0bd292252e327bcd59c4a2b62f292331a8 Mon Sep 17 00:00:00 2001 From: Alessandro Affinito Date: Fri, 22 May 2026 11:38:34 +0200 Subject: [PATCH 09/11] test(hostedcluster): fix trailing newline in predicate tests Co-Authored-By: Claude Opus 4.6 (1M context) --- .../controllers/hostedcluster/hostedcluster_predicates_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go index 3f29a2a9e2e..ff222b13325 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates_test.go @@ -378,4 +378,3 @@ func newHostedClusterForPredicateTests(generation int64, annotations map[string] }, } } - From 4030c3ec0520d7bdea91366749596f1406f836e7 Mon Sep 17 00:00:00 2001 From: Alessandro Affinito Date: Fri, 22 May 2026 12:07:52 +0200 Subject: [PATCH 10/11] api(hostedcluster): promote OCM label prefix to API package constant Move the hardcoded "api.openshift.com/" label prefix to the API package as OCMLabelPrefix so consumers outside the controller (ARO-HCP, OCM) have a single source of truth for the prefix used to mirror labels from HostedCluster to HostedControlPlane. Co-Authored-By: Claude Opus 4.6 (1M context) --- api/hypershift/v1beta1/hostedcluster_types.go | 4 ++++ .../controllers/hostedcluster/hostedcluster_predicates.go | 2 +- .../hypershift/api/hypershift/v1beta1/hostedcluster_types.go | 4 ++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/api/hypershift/v1beta1/hostedcluster_types.go b/api/hypershift/v1beta1/hostedcluster_types.go index c2d63498870..849ca5d9643 100644 --- a/api/hypershift/v1beta1/hostedcluster_types.go +++ b/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. diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go index 91461dfdf3e..c9a5084ead4 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_predicates.go @@ -94,7 +94,7 @@ var hostedClusterAdditionalActionableAnnotations = []string{ } var hostedClusterActionableLabelPrefixes = []string{ - "api.openshift.com", + hyperv1.OCMLabelPrefix, } func hostedClusterPrimaryPredicate(r client.Reader) predicate.Predicate { diff --git a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.go b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.go index c2d63498870..849ca5d9643 100644 --- a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.go +++ b/vendor/github.com/openshift/hypershift/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. From c081f95207daa8c6002c1ece8a391110b0ac751e Mon Sep 17 00:00:00 2001 From: Alessandro Affinito Date: Fri, 22 May 2026 12:07:59 +0200 Subject: [PATCH 11/11] api(hostedcluster): promote ReleaseImageValidationSkippedReason to API package Move releaseImageValidationSkippedReason from a controller-private constant to the API package as ReleaseImageValidationSkippedReason, alongside the existing AsExpectedReason and InvalidImageReason. This allows external consumers (ARO-HCP, OCM) to inspect the ValidReleaseImage condition reason programmatically without hardcoding the string. Co-Authored-By: Claude Opus 4.6 (1M context) --- api/hypershift/v1beta1/hostedcluster_conditions.go | 1 + .../controllers/hostedcluster/hostedcluster_controller.go | 7 +++---- .../hostedcluster_release_image_validation_test.go | 4 ++-- .../api/hypershift/v1beta1/hostedcluster_conditions.go | 1 + 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/api/hypershift/v1beta1/hostedcluster_conditions.go b/api/hypershift/v1beta1/hostedcluster_conditions.go index e9e01de0acc..a8e46558c9e 100644 --- a/api/hypershift/v1beta1/hostedcluster_conditions.go +++ b/api/hypershift/v1beta1/hostedcluster_conditions.go @@ -285,6 +285,7 @@ const ( ProxyCABundleInvalidReason = "ProxyCABundleInvalid" PlatformCredentialsNotFoundReason = "PlatformCredentialsNotFound" InvalidImageReason = "InvalidImage" + ReleaseImageValidationSkippedReason = "ReleaseImageValidationSkipped" InvalidIdentityProvider = "InvalidIdentityProvider" PayloadArchNotFoundReason = "PayloadArchNotFound" diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index c9f6dfc8dd1..b13d16c38bc 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -146,7 +146,6 @@ const ( previouslySyncedRestartDateAnnotation = "hypershift.openshift.io/previous-restart-date" kasServingCertHashAnnotation = "hypershift.openshift.io/kas-serving-cert-hash" referencedResourceAnnotationPrefix = "referenced-resource.hypershift.openshift.io/" - releaseImageValidationSkippedReason = "ReleaseImageValidationSkipped" ) var ( @@ -1202,7 +1201,7 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques case skipReleaseImageValidation: condition.Status = metav1.ConditionTrue condition.Message = "Release image validation is skipped by annotation" - condition.Reason = releaseImageValidationSkippedReason + condition.Reason = hyperv1.ReleaseImageValidationSkippedReason case err != nil: condition.Status = metav1.ConditionFalse condition.Message = err.Error() @@ -3750,10 +3749,10 @@ func shouldValidateReleaseImage(hcluster *hyperv1.HostedCluster, condition *meta skipReleaseImageValidation := hasSkipReleaseImageValidationAnnotation(hcluster) if skipReleaseImageValidation { - return condition.Reason != releaseImageValidationSkippedReason + return condition.Reason != hyperv1.ReleaseImageValidationSkippedReason } - return condition.Reason == releaseImageValidationSkippedReason + return condition.Reason == hyperv1.ReleaseImageValidationSkippedReason } func hasSkipReleaseImageValidationAnnotation(hcluster *hyperv1.HostedCluster) bool { diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_release_image_validation_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_release_image_validation_test.go index 2ae15ec4939..5ec3777e3e0 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_release_image_validation_test.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_release_image_validation_test.go @@ -64,7 +64,7 @@ func TestShouldValidateReleaseImage_WhenReleaseImageValidationInputsChangeItShou Type: string(hyperv1.ValidReleaseImage), Status: metav1.ConditionTrue, ObservedGeneration: 3, - Reason: releaseImageValidationSkippedReason, + Reason: hyperv1.ReleaseImageValidationSkippedReason, }, expected: false, }, @@ -77,7 +77,7 @@ func TestShouldValidateReleaseImage_WhenReleaseImageValidationInputsChangeItShou Type: string(hyperv1.ValidReleaseImage), Status: metav1.ConditionTrue, ObservedGeneration: 3, - Reason: releaseImageValidationSkippedReason, + Reason: hyperv1.ReleaseImageValidationSkippedReason, }, expected: true, }, diff --git a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go index e9e01de0acc..a8e46558c9e 100644 --- a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go +++ b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go @@ -285,6 +285,7 @@ const ( ProxyCABundleInvalidReason = "ProxyCABundleInvalid" PlatformCredentialsNotFoundReason = "PlatformCredentialsNotFound" InvalidImageReason = "InvalidImage" + ReleaseImageValidationSkippedReason = "ReleaseImageValidationSkipped" InvalidIdentityProvider = "InvalidIdentityProvider" PayloadArchNotFoundReason = "PayloadArchNotFound"