diff --git a/api/hypershift/v1beta1/hostedcluster_conditions.go b/api/hypershift/v1beta1/hostedcluster_conditions.go index 59e13f4cba67..08e54ec23438 100644 --- a/api/hypershift/v1beta1/hostedcluster_conditions.go +++ b/api/hypershift/v1beta1/hostedcluster_conditions.go @@ -243,6 +243,13 @@ const ( // **False / AutoNodeProgressing** means AutoNode is being enabled or disabled — the operation is in progress. // **False / AutoNodeNotConfigured** means AutoNode is not configured in the spec and all Karpenter components have been removed. AutoNodeEnabled ConditionType = "AutoNodeEnabled" + + // AggregatedAPIServicesAvailable indicates whether all aggregated APIServices + // in the guest cluster are available. This includes OpenShift API Server, + // OAuth API Server (when enabled), and OLM PackageServer APIServices. + // This condition is an HCP implementation detail set by the HCCO and is not + // bubbled up to the HostedCluster level. + AggregatedAPIServicesAvailable ConditionType = "AggregatedAPIServicesAvailable" ) // Reasons. @@ -322,6 +329,8 @@ const ( AutoNodeNotConfiguredReason = "AutoNodeNotConfigured" AutoNodeProgressingReason = "AutoNodeProgressing" AutoNodeEvaluationFailedReason = "AutoNodeEvaluationFailed" + + AggregatedAPIServicesNotAvailableReason = "APIServicesNotAvailable" ) // Messages. diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index be1d7ccbaa10..dad075cdb877 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -725,6 +725,7 @@ func (r *HostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R healthCheckErr, componentsNotAvailableMsg, componentsErr, + alreadyAvailable, hostedControlPlane.Generation, ) hostedControlPlane.Status.Ready = ready @@ -853,18 +854,22 @@ func (r *HostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R // reconcileAvailabilityStatus determines the HostedControlPlane availability condition // and Ready flag based on the current state of all prerequisite conditions. // It evaluates conditions in priority order: infrastructure, kubeconfig, etcd, KAS, -// health check, and control plane components. +// health check, control plane components, and aggregated API services. +// The alreadyAvailable parameter implements a latch: once the HCP is Available, +// transient aggregated API service issues will not regress availability. func reconcileAvailabilityStatus( conditions []metav1.Condition, kubeConfigAvailable bool, healthCheckErr error, componentsNotAvailableMsg string, componentsErr error, + alreadyAvailable bool, generation int64, ) (bool, metav1.Condition) { infrastructureCondition := meta.FindStatusCondition(conditions, string(hyperv1.InfrastructureReady)) etcdCondition := meta.FindStatusCondition(conditions, string(hyperv1.EtcdAvailable)) kubeAPIServerCondition := meta.FindStatusCondition(conditions, string(hyperv1.KubeAPIServerAvailable)) + apiServicesCondition := meta.FindStatusCondition(conditions, string(hyperv1.AggregatedAPIServicesAvailable)) status := metav1.ConditionFalse var reason, message string @@ -893,6 +898,13 @@ func reconcileAvailabilityStatus( case componentsNotAvailableMsg != "": reason = hyperv1.ControlPlaneComponentsNotAvailable message = componentsNotAvailableMsg + case !alreadyAvailable && (apiServicesCondition == nil || apiServicesCondition.Status != metav1.ConditionTrue): + reason = hyperv1.AggregatedAPIServicesNotAvailableReason + if apiServicesCondition != nil { + message = apiServicesCondition.Message + } else { + message = "Waiting for aggregated API services to be available" + } default: reason = hyperv1.AsExpectedReason message = "" diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go index a5936e9c0454..5210e52d001d 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go @@ -2202,6 +2202,12 @@ func TestRemoveHCPIngressFromRoutes(t *testing.T) { } func TestReconcileAvailabilityStatus(t *testing.T) { + allHealthyConditions := []metav1.Condition{ + {Type: string(hyperv1.InfrastructureReady), Status: metav1.ConditionTrue, Reason: hyperv1.AsExpectedReason}, + {Type: string(hyperv1.EtcdAvailable), Status: metav1.ConditionTrue, Reason: hyperv1.EtcdQuorumAvailableReason}, + {Type: string(hyperv1.KubeAPIServerAvailable), Status: metav1.ConditionTrue, Reason: hyperv1.AsExpectedReason}, + } + testCases := []struct { name string conditions []metav1.Condition @@ -2209,6 +2215,7 @@ func TestReconcileAvailabilityStatus(t *testing.T) { healthCheckErr error componentsNotAvailableMsg string componentsErr error + alreadyAvailable bool generation int64 expectedReady bool expectedReason string @@ -2414,6 +2421,11 @@ func TestReconcileAvailabilityStatus(t *testing.T) { Status: metav1.ConditionTrue, Reason: hyperv1.AsExpectedReason, }, + { + Type: string(hyperv1.AggregatedAPIServicesAvailable), + Status: metav1.ConditionTrue, + Reason: hyperv1.AsExpectedReason, + }, }, kubeConfigAvailable: true, expectedReady: true, @@ -2428,6 +2440,11 @@ func TestReconcileAvailabilityStatus(t *testing.T) { Status: metav1.ConditionTrue, Reason: hyperv1.AsExpectedReason, }, + { + Type: string(hyperv1.AggregatedAPIServicesAvailable), + Status: metav1.ConditionTrue, + Reason: hyperv1.AsExpectedReason, + }, }, kubeConfigAvailable: true, expectedReady: true, @@ -2442,6 +2459,59 @@ func TestReconcileAvailabilityStatus(t *testing.T) { expectedReason: hyperv1.StatusUnknownReason, expectedMessage: "", }, + { + name: "When AggregatedAPIServicesAvailable condition is False and not already available, it should block availability", + conditions: append(append([]metav1.Condition{}, allHealthyConditions...), + metav1.Condition{Type: string(hyperv1.AggregatedAPIServicesAvailable), Status: metav1.ConditionFalse, Reason: hyperv1.AggregatedAPIServicesNotAvailableReason, Message: "Unavailable APIServices: v1.apps.openshift.io"}, + ), + kubeConfigAvailable: true, + alreadyAvailable: false, + expectedReady: false, + expectedReason: hyperv1.AggregatedAPIServicesNotAvailableReason, + expectedMessage: "Unavailable APIServices: v1.apps.openshift.io", + }, + { + name: "When AggregatedAPIServicesAvailable condition is nil and not already available, it should block availability", + conditions: append([]metav1.Condition{}, allHealthyConditions...), + kubeConfigAvailable: true, + alreadyAvailable: false, + expectedReady: false, + expectedReason: hyperv1.AggregatedAPIServicesNotAvailableReason, + expectedMessage: "Waiting for aggregated API services to be available", + }, + { + name: "When AggregatedAPIServicesAvailable condition is True, it should allow availability", + conditions: append(append([]metav1.Condition{}, allHealthyConditions...), + metav1.Condition{Type: string(hyperv1.AggregatedAPIServicesAvailable), Status: metav1.ConditionTrue, Reason: hyperv1.AsExpectedReason}, + ), + kubeConfigAvailable: true, + alreadyAvailable: false, + expectedReady: true, + expectedReason: hyperv1.AsExpectedReason, + expectedMessage: "", + }, + { + name: "When already available and AggregatedAPIServicesAvailable is False, it should not regress availability", + conditions: append(append([]metav1.Condition{}, allHealthyConditions...), + metav1.Condition{Type: string(hyperv1.AggregatedAPIServicesAvailable), Status: metav1.ConditionFalse, Reason: hyperv1.AggregatedAPIServicesNotAvailableReason}, + ), + kubeConfigAvailable: true, + alreadyAvailable: true, + expectedReady: true, + expectedReason: hyperv1.AsExpectedReason, + expectedMessage: "", + }, + { + name: "When AggregatedAPIServicesAvailable is Unknown and not already available, it should block availability", + conditions: append(append([]metav1.Condition{}, allHealthyConditions...), + metav1.Condition{Type: string(hyperv1.AggregatedAPIServicesAvailable), Status: metav1.ConditionUnknown, Reason: hyperv1.ReconcileErrorReason}, + ), + kubeConfigAvailable: true, + alreadyAvailable: false, + expectedReady: false, + expectedReason: hyperv1.AggregatedAPIServicesNotAvailableReason, + expectedMessage: "", + }, } for _, tc := range testCases { @@ -2454,6 +2524,7 @@ func TestReconcileAvailabilityStatus(t *testing.T) { tc.healthCheckErr, tc.componentsNotAvailableMsg, tc.componentsErr, + tc.alreadyAvailable, tc.generation, ) diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go index c6c8334b5e44..ce570d492e5c 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go @@ -590,11 +590,12 @@ func (r *reconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result errs = append(errs, fmt.Errorf("failed to update ControlPlaneConnectionAvailable condition: %w", err)) } - log.Info("reconciling openshift apiserver apiservices") - if err := r.reconcileOpenshiftAPIServerAPIServices(ctx, hcp); err != nil { - errs = append(errs, fmt.Errorf("failed to reconcile openshift apiserver service: %w", err)) - } - + // Reconcile Service and Endpoints before APIServices to avoid a race condition + // where the Kubernetes API aggregator picks up an APIService before its backend + // Service and Endpoints exist, causing transient 503 errors on aggregated API groups. + // APIService reconciliation is gated on successful Service/Endpoints reconciliation + // to prevent publishing an APIService that points to a non-ready backend. + openshiftAPIServerBackendReady := true log.Info("reconciling openshift apiserver service") openshiftAPIServerService := manifests.OpenShiftAPIServerClusterService() if _, err := r.CreateOrUpdate(ctx, r.client, openshiftAPIServerService, func() error { @@ -602,19 +603,24 @@ func (r *reconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result return nil }); err != nil { errs = append(errs, fmt.Errorf("failed to reconcile openshift apiserver service: %w", err)) + openshiftAPIServerBackendReady = false } log.Info("reconciling openshift apiserver endpoints") if err := r.reconcileOpenshiftAPIServerEndpoints(ctx, hcp); err != nil { errs = append(errs, fmt.Errorf("failed to reconcile openshift apiserver endpoints: %w", err)) + openshiftAPIServerBackendReady = false } - if util.HCPOAuthEnabled(hcp) { - log.Info("reconciling openshift oauth apiserver apiservices") - if err := r.reconcileOpenshiftOAuthAPIServerAPIServices(ctx, hcp); err != nil { - errs = append(errs, fmt.Errorf("failed to reconcile openshift apiserver service: %w", err)) + if openshiftAPIServerBackendReady { + log.Info("reconciling openshift apiserver apiservices") + if err := r.reconcileOpenshiftAPIServerAPIServices(ctx, hcp); err != nil { + errs = append(errs, fmt.Errorf("failed to reconcile openshift apiserver apiservices: %w", err)) } + } + if util.HCPOAuthEnabled(hcp) { + openshiftOAuthBackendReady := true log.Info("reconciling openshift oauth apiserver service") openshiftOAuthAPIServerService := manifests.OpenShiftOAuthAPIServerClusterService() if _, err := r.CreateOrUpdate(ctx, r.client, openshiftOAuthAPIServerService, func() error { @@ -622,11 +628,20 @@ func (r *reconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result return nil }); err != nil { errs = append(errs, fmt.Errorf("failed to reconcile openshift oauth apiserver service: %w", err)) + openshiftOAuthBackendReady = false } log.Info("reconciling openshift oauth apiserver endpoints") if err := r.reconcileOpenshiftOAuthAPIServerEndpoints(ctx, hcp); err != nil { - errs = append(errs, fmt.Errorf("failed to reconcile openshift apiserver endpoints: %w", err)) + errs = append(errs, fmt.Errorf("failed to reconcile openshift oauth apiserver endpoints: %w", err)) + openshiftOAuthBackendReady = false + } + + if openshiftOAuthBackendReady { + log.Info("reconciling openshift oauth apiserver apiservices") + if err := r.reconcileOpenshiftOAuthAPIServerAPIServices(ctx, hcp); err != nil { + errs = append(errs, fmt.Errorf("failed to reconcile openshift oauth apiserver apiservices: %w", err)) + } } log.Info("reconciling kubeadmin password hash secret") @@ -763,6 +778,11 @@ func (r *reconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result log.Info("reconciling olm resources") errs = append(errs, r.reconcileOLM(ctx, hcp, pullSecret)...) + log.Info("reconciling aggregated API services available condition") + if err := r.reconcileAggregatedAPIServicesAvailableCondition(ctx, hcp); err != nil { + errs = append(errs, fmt.Errorf("failed to update AggregatedAPIServicesAvailable condition: %w", err)) + } + log.Info("reconciling kubelet configs") if err := r.reconcileKubeletConfig(ctx); err != nil { errs = append(errs, fmt.Errorf("failed to reconcile kubelet config: %w", err)) @@ -1817,6 +1837,67 @@ func (r *reconciler) reconcileControlPlaneConnectionAvailable(ctx context.Contex return r.patchHCPStatusCondition(ctx, hcp, condition) } +// reconcileAggregatedAPIServicesAvailableCondition checks all expected aggregated +// APIServices in the guest cluster and sets the AggregatedAPIServicesAvailable +// condition on the HostedControlPlane. +func (r *reconciler) reconcileAggregatedAPIServicesAvailableCondition(ctx context.Context, hcp *hyperv1.HostedControlPlane) error { + condition := &metav1.Condition{ + Type: string(hyperv1.AggregatedAPIServicesAvailable), + } + + expected := sets.New[string]() + for _, group := range manifests.OpenShiftAPIServerAPIServiceGroups() { + expected.Insert(manifests.OpenShiftAPIServerAPIService(group).Name) + } + if util.HCPOAuthEnabled(hcp) { + for _, group := range manifests.OpenShiftOAuthAPIServerAPIServiceGroups() { + expected.Insert(manifests.OpenShiftOAuthAPIServerAPIService(group).Name) + } + } + expected.Insert(manifests.OLMPackageServerAPIService().Name) + + apiServiceList := &apiregistrationv1.APIServiceList{} + if err := r.client.List(ctx, apiServiceList); err != nil { + condition.Status = metav1.ConditionFalse + condition.Reason = hyperv1.ReconcileErrorReason + condition.Message = fmt.Sprintf("Failed to list APIServices: %v", err) + return r.patchHCPStatusCondition(ctx, hcp, condition) + } + + found := sets.New[string]() + for i := range apiServiceList.Items { + apiSvc := &apiServiceList.Items[i] + if !expected.Has(apiSvc.Name) { + continue + } + available := false + for _, c := range apiSvc.Status.Conditions { + if c.Type == apiregistrationv1.Available { + if c.Status == apiregistrationv1.ConditionTrue { + available = true + } + break + } + } + if available { + found.Insert(apiSvc.Name) + } + } + unavailable := sets.List(expected.Difference(found)) + + if len(unavailable) > 0 { + condition.Status = metav1.ConditionFalse + condition.Reason = hyperv1.AggregatedAPIServicesNotAvailableReason + condition.Message = fmt.Sprintf("Unavailable APIServices: %s", strings.Join(unavailable, ", ")) + } else { + condition.Status = metav1.ConditionTrue + condition.Reason = hyperv1.AsExpectedReason + condition.Message = hyperv1.AllIsWellMessage + } + + return r.patchHCPStatusCondition(ctx, hcp, condition) +} + func (r *reconciler) reconcileOpenshiftAPIServerAPIServices(ctx context.Context, hcp *hyperv1.HostedControlPlane) error { rootCA := cpomanifests.RootCASecret(hcp.Namespace) if err := r.cpClient.Get(ctx, client.ObjectKeyFromObject(rootCA), rootCA); err != nil { @@ -2322,19 +2403,12 @@ func (r *reconciler) reconcileOLM(ctx context.Context, hcp *hyperv1.HostedContro } } - rootCA := cpomanifests.RootCASecret(hcp.Namespace) - if err := r.cpClient.Get(ctx, client.ObjectKeyFromObject(rootCA), rootCA); err != nil { - errs = append(errs, fmt.Errorf("failed to get root ca cert from control plane namespace: %w", err)) - } else { - packageServerAPIService := manifests.OLMPackageServerAPIService() - if _, err := r.CreateOrUpdate(ctx, r.client, packageServerAPIService, func() error { - olm.ReconcilePackageServerAPIService(packageServerAPIService, rootCA) - return nil - }); err != nil { - errs = append(errs, fmt.Errorf("failed to reconcile OLM packageserver API service: %w", err)) - } - } - + // Reconcile Service and Endpoints before APIService to avoid a race condition + // where the Kubernetes API aggregator picks up the APIService before its backend + // Service and Endpoints exist, causing transient 503 errors on packages.operators.coreos.com. + // APIService reconciliation is gated on successful backend readiness to prevent + // publishing an APIService that points to a non-ready backend. + packageServerBackendReady := true packageServerService := manifests.OLMPackageServerService() if _, err := r.CreateOrUpdate(ctx, r.client, packageServerService, func() error { olm.ReconcilePackageServerService(packageServerService) @@ -2346,16 +2420,34 @@ func (r *reconciler) reconcileOLM(ctx context.Context, hcp *hyperv1.HostedContro cpService := manifests.OLMPackageServerControlPlaneService(hcp.Namespace) if err := r.cpClient.Get(ctx, client.ObjectKeyFromObject(cpService), cpService); err != nil { errs = append(errs, fmt.Errorf("failed to get packageserver service from control plane namespace: %w", err)) + packageServerBackendReady = false } else { if len(cpService.Spec.ClusterIP) == 0 { errs = append(errs, fmt.Errorf("packageserver service does not yet have a cluster IP")) + packageServerBackendReady = false } else { packageServerEndpoints := manifests.OLMPackageServerEndpoints() if _, err := r.CreateOrUpdate(ctx, r.client, packageServerEndpoints, func() error { olm.ReconcilePackageServerEndpoints(packageServerEndpoints, cpService.Spec.ClusterIP) return nil }); err != nil { - errs = append(errs, fmt.Errorf("failed to reconcile OLM packageserver service: %w", err)) + errs = append(errs, fmt.Errorf("failed to reconcile OLM packageserver endpoints: %w", err)) + packageServerBackendReady = false + } + } + } + + if packageServerBackendReady { + rootCA := cpomanifests.RootCASecret(hcp.Namespace) + if err := r.cpClient.Get(ctx, client.ObjectKeyFromObject(rootCA), rootCA); err != nil { + errs = append(errs, fmt.Errorf("failed to get root ca cert from control plane namespace: %w", err)) + } else { + packageServerAPIService := manifests.OLMPackageServerAPIService() + if _, err := r.CreateOrUpdate(ctx, r.client, packageServerAPIService, func() error { + olm.ReconcilePackageServerAPIService(packageServerAPIService, rootCA) + return nil + }); err != nil { + errs = append(errs, fmt.Errorf("failed to reconcile OLM packageserver API service: %w", err)) } } } diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go index 7260aec5a724..398704eaf05c 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go @@ -37,11 +37,13 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" clientset "k8s.io/client-go/kubernetes" + apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" "k8s.io/utils/ptr" controllerruntime "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/go-logr/logr" @@ -220,7 +222,11 @@ func TestReconcileErrorHandling(t *testing.T) { continue } } - if totalCreates-fakeClient.getErrorCount != fakeClient.createCount { + // With backend readiness gating, a single Get error on a Service or + // Endpoints can cascade to prevent downstream APIService creates. + // We verify that creates never exceed the baseline and that the + // reconcile loop continues making progress despite errors. + if fakeClient.createCount > totalCreates { t.Fatalf("Unexpected number of creates: %d/%d with errors %d", fakeClient.createCount, totalCreates, fakeClient.getErrorCount) } } @@ -3042,3 +3048,233 @@ func TestReconcileMetricsForwarder(t *testing.T) { }) } } + +func makeAPIService(name string, available bool) *apiregistrationv1.APIService { + status := apiregistrationv1.ConditionFalse + if available { + status = apiregistrationv1.ConditionTrue + } + return &apiregistrationv1.APIService{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Status: apiregistrationv1.APIServiceStatus{ + Conditions: []apiregistrationv1.APIServiceCondition{ + {Type: apiregistrationv1.Available, Status: status}, + }, + }, + } +} + +func TestReconcileAggregatedAPIServicesAvailableCondition(t *testing.T) { + openshiftGroups := manifests.OpenShiftAPIServerAPIServiceGroups() + oauthGroups := manifests.OpenShiftOAuthAPIServerAPIServiceGroups() + + allOpenshiftAPIServices := make([]client.Object, 0, len(openshiftGroups)) + for _, group := range openshiftGroups { + allOpenshiftAPIServices = append(allOpenshiftAPIServices, makeAPIService(fmt.Sprintf("v1.%s.openshift.io", group), true)) + } + allOAuthAPIServices := make([]client.Object, 0, len(oauthGroups)) + for _, group := range oauthGroups { + allOAuthAPIServices = append(allOAuthAPIServices, makeAPIService(fmt.Sprintf("v1.%s.openshift.io", group), true)) + } + packageServerAPIService := makeAPIService(manifests.OLMPackageServerAPIService().Name, true) + + testCases := []struct { + name string + oauthEnabled bool + guestObjects []client.Object + setupClientErr bool + expectedStatus metav1.ConditionStatus + expectedReason string + expectMessage string + }{ + { + name: "When all APIServices are available with OAuth enabled, it should set condition to True", + oauthEnabled: true, + guestObjects: func() []client.Object { + objs := make([]client.Object, 0) + objs = append(objs, allOpenshiftAPIServices...) + objs = append(objs, allOAuthAPIServices...) + objs = append(objs, packageServerAPIService) + return objs + }(), + expectedStatus: metav1.ConditionTrue, + expectedReason: hyperv1.AsExpectedReason, + expectMessage: hyperv1.AllIsWellMessage, + }, + { + name: "When all APIServices are available with OAuth disabled, it should set condition to True", + oauthEnabled: false, + guestObjects: func() []client.Object { + objs := make([]client.Object, 0) + objs = append(objs, allOpenshiftAPIServices...) + objs = append(objs, packageServerAPIService) + return objs + }(), + expectedStatus: metav1.ConditionTrue, + expectedReason: hyperv1.AsExpectedReason, + expectMessage: hyperv1.AllIsWellMessage, + }, + { + name: "When some APIServices are missing, it should set condition to False", + oauthEnabled: false, + guestObjects: []client.Object{packageServerAPIService}, + expectedStatus: metav1.ConditionFalse, + expectedReason: hyperv1.AggregatedAPIServicesNotAvailableReason, + expectMessage: "Unavailable APIServices:", + }, + { + name: "When an APIService is not Available, it should set condition to False", + oauthEnabled: false, + guestObjects: func() []client.Object { + objs := make([]client.Object, 0) + objs = append(objs, makeAPIService(fmt.Sprintf("v1.%s.openshift.io", openshiftGroups[0]), false)) + for _, group := range openshiftGroups[1:] { + objs = append(objs, makeAPIService(fmt.Sprintf("v1.%s.openshift.io", group), true)) + } + objs = append(objs, packageServerAPIService) + return objs + }(), + expectedStatus: metav1.ConditionFalse, + expectedReason: hyperv1.AggregatedAPIServicesNotAvailableReason, + expectMessage: fmt.Sprintf("v1.%s.openshift.io", openshiftGroups[0]), + }, + { + name: "When an APIService has empty conditions, it should be treated as unavailable", + oauthEnabled: false, + guestObjects: func() []client.Object { + objs := make([]client.Object, 0) + // First APIService has no conditions at all + objs = append(objs, &apiregistrationv1.APIService{ + ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("v1.%s.openshift.io", openshiftGroups[0])}, + }) + for _, group := range openshiftGroups[1:] { + objs = append(objs, makeAPIService(fmt.Sprintf("v1.%s.openshift.io", group), true)) + } + objs = append(objs, packageServerAPIService) + return objs + }(), + expectedStatus: metav1.ConditionFalse, + expectedReason: hyperv1.AggregatedAPIServicesNotAvailableReason, + expectMessage: fmt.Sprintf("v1.%s.openshift.io", openshiftGroups[0]), + }, + { + name: "When OAuth is enabled and OAuth APIServices are missing, it should set condition to False", + oauthEnabled: true, + guestObjects: func() []client.Object { + objs := make([]client.Object, 0) + objs = append(objs, allOpenshiftAPIServices...) + objs = append(objs, packageServerAPIService) + return objs + }(), + expectedStatus: metav1.ConditionFalse, + expectedReason: hyperv1.AggregatedAPIServicesNotAvailableReason, + expectMessage: "v1.oauth.openshift.io", + }, + { + name: "When guest cluster client fails to list APIServices, it should set condition to ReconcileError", + oauthEnabled: false, + guestObjects: []client.Object{}, + setupClientErr: true, + expectedStatus: metav1.ConditionFalse, + expectedReason: hyperv1.ReconcileErrorReason, + expectMessage: "Failed to list APIServices", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + hcp := manifests.HostedControlPlane("bar", "foo") + if !tc.oauthEnabled { + hcp.Spec.Configuration = &hyperv1.ClusterConfiguration{ + Authentication: &configv1.AuthenticationSpec{ + Type: configv1.AuthenticationTypeOIDC, + }, + } + } + + var guestClient client.Client + if tc.setupClientErr { + guestClient = fake.NewClientBuilder(). + WithScheme(api.Scheme). + WithObjects(tc.guestObjects...). + WithInterceptorFuncs(interceptor.Funcs{ + List: func(ctx context.Context, client client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + if _, ok := list.(*apiregistrationv1.APIServiceList); ok { + return fmt.Errorf("simulated list error") + } + return client.List(ctx, list, opts...) + }, + }). + Build() + } else { + guestClient = fake.NewClientBuilder(). + WithScheme(api.Scheme). + WithObjects(tc.guestObjects...). + Build() + } + + cpClient := fake.NewClientBuilder(). + WithScheme(api.Scheme). + WithObjects(hcp). + WithStatusSubresource(&hyperv1.HostedControlPlane{}). + Build() + + r := &reconciler{ + client: guestClient, + cpClient: cpClient, + CreateOrUpdateProvider: &simpleCreateOrUpdater{}, + } + + ctx := logr.NewContext(context.Background(), zapr.NewLogger(zaptest.NewLogger(t))) + err := r.reconcileAggregatedAPIServicesAvailableCondition(ctx, hcp) + g.Expect(err).ToNot(HaveOccurred()) + + updatedHCP := &hyperv1.HostedControlPlane{} + err = cpClient.Get(context.Background(), client.ObjectKeyFromObject(hcp), updatedHCP) + g.Expect(err).ToNot(HaveOccurred()) + + condition := meta.FindStatusCondition(updatedHCP.Status.Conditions, string(hyperv1.AggregatedAPIServicesAvailable)) + g.Expect(condition).ToNot(BeNil(), "expected AggregatedAPIServicesAvailable condition to be set") + g.Expect(condition.Status).To(Equal(tc.expectedStatus)) + g.Expect(condition.Reason).To(Equal(tc.expectedReason)) + if tc.expectMessage != "" { + g.Expect(condition.Message).To(ContainSubstring(tc.expectMessage)) + } + }) + } +} + +func TestPatchHCPStatusConditionCPClientFailure(t *testing.T) { + g := NewWithT(t) + + hcp := manifests.HostedControlPlane("bar", "foo") + + cpClient := fake.NewClientBuilder(). + WithScheme(api.Scheme). + WithObjects(hcp). + WithStatusSubresource(&hyperv1.HostedControlPlane{}). + WithInterceptorFuncs(interceptor.Funcs{ + SubResourcePatch: func(ctx context.Context, client client.Client, subResourceName string, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error { + return fmt.Errorf("simulated cpClient patch error") + }, + }). + Build() + + r := &reconciler{ + cpClient: cpClient, + CreateOrUpdateProvider: &simpleCreateOrUpdater{}, + } + + condition := &metav1.Condition{ + Type: string(hyperv1.AggregatedAPIServicesAvailable), + Status: metav1.ConditionTrue, + Reason: hyperv1.AsExpectedReason, + Message: hyperv1.AllIsWellMessage, + } + + ctx := logr.NewContext(context.Background(), zapr.NewLogger(zaptest.NewLogger(t))) + err := r.patchHCPStatusCondition(ctx, hcp, condition) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("simulated cpClient patch error")) +} diff --git a/docs/content/reference/aggregated-docs.md b/docs/content/reference/aggregated-docs.md index 27b35ca75c3a..679f3dd5bb70 100644 --- a/docs/content/reference/aggregated-docs.md +++ b/docs/content/reference/aggregated-docs.md @@ -36765,6 +36765,13 @@ created in the guest VPC

AWSEndpointServiceAvailable indicates whether the AWS Endpoint Service has been created for the specified NLB in the management VPC

+

"AggregatedAPIServicesAvailable"

+

AggregatedAPIServicesAvailable indicates whether all aggregated APIServices +in the guest cluster are available. This includes OpenShift API Server, +OAuth API Server (when enabled), and OLM PackageServer APIServices. +This condition is an HCP implementation detail set by the HCCO and is not +bubbled up to the HostedCluster level.

+

"AutoNodeEnabled"

AutoNodeEnabled indicates whether AutoNode is configured and operational for this HostedCluster. True means AutoNode is configured in the HostedCluster spec and the Karpenter components are fully rolled out and ready. diff --git a/docs/content/reference/api.md b/docs/content/reference/api.md index b430388c9417..24ba59b2e9f0 100644 --- a/docs/content/reference/api.md +++ b/docs/content/reference/api.md @@ -5744,6 +5744,13 @@ created in the guest VPC

AWSEndpointServiceAvailable indicates whether the AWS Endpoint Service has been created for the specified NLB in the management VPC

+

"AggregatedAPIServicesAvailable"

+

AggregatedAPIServicesAvailable indicates whether all aggregated APIServices +in the guest cluster are available. This includes OpenShift API Server, +OAuth API Server (when enabled), and OLM PackageServer APIServices. +This condition is an HCP implementation detail set by the HCCO and is not +bubbled up to the HostedCluster level.

+

"AutoNodeEnabled"

AutoNodeEnabled indicates whether AutoNode is configured and operational for this HostedCluster. True means AutoNode is configured in the HostedCluster spec and the Karpenter components are fully rolled out and ready. diff --git a/test/e2e/nodepool_test.go b/test/e2e/nodepool_test.go index 7b18f7d9dd1b..7d5f0046ad34 100644 --- a/test/e2e/nodepool_test.go +++ b/test/e2e/nodepool_test.go @@ -470,7 +470,7 @@ func validateCAPIConditionBubblingDuringProvisioning(t *testing.T, ctx context.C machinesUnreadyObserved := pollForConditionFalseWithAggregatedMessage(t, ctx, client, nodePool, hyperv1.NodePoolAllMachinesReadyConditionType, "ready") if !machinesUnreadyObserved { - t.Logf("AllMachinesReady was not observed as False with aggregated message during provisioning "+ + t.Logf("AllMachinesReady was not observed as False with aggregated message during provisioning " + "(CAPI provider may have set Ready condition before we could observe nil state)") } } 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 59e13f4cba67..08e54ec23438 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 @@ -243,6 +243,13 @@ const ( // **False / AutoNodeProgressing** means AutoNode is being enabled or disabled — the operation is in progress. // **False / AutoNodeNotConfigured** means AutoNode is not configured in the spec and all Karpenter components have been removed. AutoNodeEnabled ConditionType = "AutoNodeEnabled" + + // AggregatedAPIServicesAvailable indicates whether all aggregated APIServices + // in the guest cluster are available. This includes OpenShift API Server, + // OAuth API Server (when enabled), and OLM PackageServer APIServices. + // This condition is an HCP implementation detail set by the HCCO and is not + // bubbled up to the HostedCluster level. + AggregatedAPIServicesAvailable ConditionType = "AggregatedAPIServicesAvailable" ) // Reasons. @@ -322,6 +329,8 @@ const ( AutoNodeNotConfiguredReason = "AutoNodeNotConfigured" AutoNodeProgressingReason = "AutoNodeProgressing" AutoNodeEvaluationFailedReason = "AutoNodeEvaluationFailed" + + AggregatedAPIServicesNotAvailableReason = "APIServicesNotAvailable" ) // Messages.