diff --git a/Dockerfile.rhel b/Dockerfile.rhel index ef26c34d38..9146768451 100644 --- a/Dockerfile.rhel +++ b/Dockerfile.rhel @@ -5,6 +5,7 @@ RUN make clean build FROM registry.ci.openshift.org/ocp/4.22:base-rhel9 COPY --from=builder /go/src/github.com/openshift/cluster-capi-operator/bin/capi-operator . +COPY --from=builder /go/src/github.com/openshift/cluster-capi-operator/bin/capi-installer . COPY --from=builder /go/src/github.com/openshift/cluster-capi-operator/bin/capi-controllers . COPY --from=builder /go/src/github.com/openshift/cluster-capi-operator/bin/machine-api-migration . COPY --from=builder /go/src/github.com/openshift/cluster-capi-operator/bin/crd-compatibility-checker . diff --git a/Makefile b/Makefile index 7faa6f7264..b87ed94a2e 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ IMG ?= controller:latest PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST)))) # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary. -ENVTEST_K8S_VERSION = 1.33.2 +ENVTEST_K8S_VERSION = 1.35.1 ENVTEST = go run -mod=vendor ${PROJECT_DIR}/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest GOLANGCI_LINT = go run -mod=vendor ${PROJECT_DIR}/vendor/github.com/golangci/golangci-lint/v2/cmd/golangci-lint @@ -25,11 +25,15 @@ verify-%: make $* ./hack/verify-diff.sh -verify: fmt lint verify-ocp-manifests ## Run formatting and linting checks +verify: generate fmt lint verify-ocp-manifests ## Run formatting and linting checks test: verify unit ## Run verification and unit tests -build: bin/capi-operator bin/capi-controllers bin/machine-api-migration bin/crd-compatibility-checker manifests-gen ## Build all binaries +.PHONY: generate +generate: + go generate ./... + +build: generate bin/capi-operator bin/capi-installer bin/capi-controllers bin/machine-api-migration bin/crd-compatibility-checker manifests-gen ## Build all binaries clean: rm -rf bin/* diff --git a/cmd/capi-installer/main.go b/cmd/capi-installer/main.go new file mode 100644 index 0000000000..ef4b8caec2 --- /dev/null +++ b/cmd/capi-installer/main.go @@ -0,0 +1,193 @@ +// Copyright 2026 Red Hat, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "context" + "errors" + "flag" + "fmt" + "os" + "time" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/sets" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/ptr" + + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/cache" + + configv1 "github.com/openshift/api/config/v1" + operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1" + + "github.com/openshift/cluster-capi-operator/pkg/commoncmdoptions" + "github.com/openshift/cluster-capi-operator/pkg/controllers" + "github.com/openshift/cluster-capi-operator/pkg/controllers/installer" + "github.com/openshift/cluster-capi-operator/pkg/controllers/revision" + "github.com/openshift/cluster-capi-operator/pkg/providerimages" + "github.com/openshift/cluster-capi-operator/pkg/util" +) + +var errPodIdentityNotSet = errors.New("POD_NAME and POD_NAMESPACE must be set") + +const ( + managerName = "capi-installer" +) + +func initScheme(scheme *runtime.Scheme) { + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(configv1.AddToScheme(scheme)) + utilruntime.Must(apiextensionsv1.AddToScheme(scheme)) + utilruntime.Must(appsv1.AddToScheme(scheme)) + utilruntime.Must(operatorv1alpha1.AddToScheme(scheme)) +} + +func main() { + ctx, cancel := context.WithCancel(ctrl.SetupSignalHandler()) + cfg := ctrl.GetConfigOrDie() + + scheme := runtime.NewScheme() + initScheme(scheme) + + extraflags := flag.NewFlagSet("", flag.ContinueOnError) + providerImageDir := extraflags.String( + "provider-image-dir", + providerimages.ProviderImageMountBase, + "Directory containing provider image manifests. In dev mode, set to a local directory to skip pod spec reading.", + ) + + log, operatorConfig, mgrOpts, initManager, err := commoncmdoptions.InitOperatorConfig(ctx, cfg, scheme, managerName, controllers.DefaultOperatorNamespace, extraflags) + if err != nil { + log.Error(err, "unable to initialize operator config") + os.Exit(1) + } + + mgrOpts.Cache = cache.Options{ + DefaultNamespaces: map[string]cache.Config{ + *operatorConfig.CAPINamespace: {}, + *operatorConfig.OperatorNamespace: {}, + }, + SyncPeriod: ptr.To(10 * time.Minute), + } + + mgr, err := initManager(ctx, cancel, mgrOpts) + if err != nil { + log.Error(err, "unable to initialize manager") + os.Exit(1) + } + + if err := setupControllers(ctx, mgr, operatorConfig, *providerImageDir); err != nil { + log.Error(err, "unable to setup controllers") + os.Exit(1) + } + + log.Info("Starting " + managerName + " manager") + + if err := mgr.Start(ctx); err != nil { + log.Error(err, "problem running manager") + os.Exit(1) + } +} + +func setupControllers(ctx context.Context, mgr ctrl.Manager, operatorConfig commoncmdoptions.OperatorConfig, providerImageDir string) error { + allProviderProfiles, err := loadProviderImages(ctx, mgr, providerImageDir) + if err != nil { + return err + } + + currentReleaseRefs, err := loadCurrentReleaseImageRefs(ctx, mgr, *operatorConfig.OperatorNamespace) + if err != nil { + return err + } + + currentReleaseProfiles := make([]providerimages.ProviderImageManifests, 0, len(allProviderProfiles)) + for _, profile := range allProviderProfiles { + if currentReleaseRefs.Has(profile.ImageRef) { + currentReleaseProfiles = append(currentReleaseProfiles, profile) + } + } + + log := ctrl.LoggerFrom(ctx) + for _, profile := range allProviderProfiles { + log.Info("loaded provider profile", "name", profile.Name, "imageRef", profile.ImageRef, "profile", profile.Profile) + } + + if err := (&revision.RevisionController{ + Client: mgr.GetClient(), + ProviderProfiles: currentReleaseProfiles, + ReleaseVersion: util.GetReleaseVersion(), + }).SetupWithManager(mgr, operatorConfig.TLSOptions); err != nil { + log.Error(err, "unable to create revision controller", "controller", "RevisionController") + return fmt.Errorf("unable to create revision controller: %w", err) + } + + if err := installer.SetupWithManager(mgr, allProviderProfiles); err != nil { + return fmt.Errorf("unable to create installer controller: %w", err) + } + + return nil +} + +func loadProviderImages(ctx context.Context, mgr ctrl.Manager, providerImageDir string) ([]providerimages.ProviderImageManifests, error) { + podName := os.Getenv("POD_NAME") + podNamespace := os.Getenv("POD_NAMESPACE") + + if podName == "" || podNamespace == "" { + return nil, errPodIdentityNotSet + } + + var pod corev1.Pod + if err := mgr.GetAPIReader().Get(ctx, types.NamespacedName{Name: podName, Namespace: podNamespace}, &pod); err != nil { + return nil, fmt.Errorf("unable to get pod %s/%s: %w", podNamespace, podName, err) + } + + imageRefMap, err := providerimages.BuildImageRefMap(pod.Spec, managerName) + if err != nil { + return nil, fmt.Errorf("unable to build image ref map from pod spec: %w", err) + } + + log := ctrl.LoggerFrom(ctx) + + providerProfiles, err := providerimages.ScanProviderImages(log, providerImageDir, imageRefMap) + if err != nil { + return nil, fmt.Errorf("unable to scan provider images: %w", err) + } + + return providerProfiles, nil +} + +func loadCurrentReleaseImageRefs(ctx context.Context, mgr ctrl.Manager, operatorNamespace string) (sets.Set[string], error) { + configMap := &corev1.ConfigMap{} + + if err := mgr.GetAPIReader().Get(ctx, types.NamespacedName{ + Name: providerimages.ConfigMapName, + Namespace: operatorNamespace, + }, configMap); err != nil { + return nil, fmt.Errorf("unable to get ConfigMap %s/%s: %w", operatorNamespace, providerimages.ConfigMapName, err) + } + + imageRefs, err := providerimages.ImageRefsFromConfigMap(configMap) + if err != nil { + return nil, fmt.Errorf("unable to extract image refs from ConfigMap: %w", err) + } + + return imageRefs, nil +} diff --git a/cmd/capi-operator/main.go b/cmd/capi-operator/main.go index d2ede0aa09..d964ffdc34 100644 --- a/cmd/capi-operator/main.go +++ b/cmd/capi-operator/main.go @@ -17,14 +17,12 @@ package main import ( "context" "errors" - "flag" "fmt" "os" "time" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -33,6 +31,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/go-logr/logr" configv1 "github.com/openshift/api/config/v1" @@ -41,24 +40,23 @@ import ( "github.com/openshift/cluster-capi-operator/pkg/commoncmdoptions" "github.com/openshift/cluster-capi-operator/pkg/controllers" "github.com/openshift/cluster-capi-operator/pkg/controllers/clusteroperator" - "github.com/openshift/cluster-capi-operator/pkg/controllers/installer" - "github.com/openshift/cluster-capi-operator/pkg/controllers/revision" - "github.com/openshift/cluster-capi-operator/pkg/providerimages" + "github.com/openshift/cluster-capi-operator/pkg/controllers/installerdeployment" "github.com/openshift/cluster-capi-operator/pkg/util" ) -var errPodIdentityNotSet = errors.New("POD_NAME and POD_NAMESPACE must be set") +var ( + errPodIdentityNotSet = errors.New("POD_NAME and POD_NAMESPACE must be set") + errContainerNotInPod = errors.New("container not found in pod spec") + errInfrastructurePlatformStatusNotSet = errors.New("infrastructure platform status is not set") +) const ( managerName = "capi-operator" - - defaultProviderImageDirPath = "/var/lib/provider-images" ) func initScheme(scheme *runtime.Scheme) { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(configv1.AddToScheme(scheme)) - utilruntime.Must(apiextensionsv1.AddToScheme(scheme)) utilruntime.Must(appsv1.AddToScheme(scheme)) utilruntime.Must(operatorv1alpha1.AddToScheme(scheme)) } @@ -70,14 +68,7 @@ func main() { scheme := runtime.NewScheme() initScheme(scheme) - extraflags := flag.NewFlagSet("", flag.ContinueOnError) - providerImageDir := extraflags.String( - "provider-image-dir", - defaultProviderImageDirPath, - "Directory containing provider image manifests. In dev mode, set to a local directory to skip pod spec reading.", - ) - - log, operatorConfig, mgrOpts, initManager, err := commoncmdoptions.InitOperatorConfig(ctx, cfg, scheme, managerName, controllers.DefaultOperatorNamespace, extraflags) + log, operatorConfig, mgrOpts, initManager, err := commoncmdoptions.InitOperatorConfig(ctx, cfg, scheme, managerName, controllers.DefaultOperatorNamespace, nil) if err != nil { log.Error(err, "unable to initialize operator config") os.Exit(1) @@ -97,7 +88,7 @@ func main() { os.Exit(1) } - if err := setupControllers(ctx, log, mgr, operatorConfig, *providerImageDir, cancel); err != nil { + if err := setupControllers(ctx, log, mgr, operatorConfig, cancel); err != nil { log.Error(err, "unable to setup controllers") os.Exit(1) } @@ -110,89 +101,70 @@ func main() { } } -func setupControllers(ctx context.Context, log logr.Logger, mgr ctrl.Manager, operatorConfig commoncmdoptions.OperatorConfig, providerImageDir string, cancel context.CancelFunc) error { +func setupControllers(ctx context.Context, log logr.Logger, mgr ctrl.Manager, operatorConfig commoncmdoptions.OperatorConfig, cancel context.CancelFunc) error { infra, err := util.GetInfra(ctx, mgr.GetAPIReader()) if err != nil { return fmt.Errorf("unable to get infrastructure: %w", err) } - platform, err := util.GetPlatformFromInfra(infra) - if err != nil { - return fmt.Errorf("unable to get platform: %w", err) - } - featureGates, err := util.GetFeatureGates(ctx, log, managerName, mgr.GetConfig(), cancel) if err != nil { return fmt.Errorf("unable to get feature gates: %w", err) } + if infra.Status.PlatformStatus == nil { + return errInfrastructurePlatformStatusNotSet + } + supportedPlatform := util.IsCAPIEnabledForPlatform(featureGates, infra.Status.PlatformStatus.Type) if err := (&clusteroperator.ClusterOperatorController{ - ClusterOperatorStatusClient: operatorConfig.GetClusterOperatorStatusClient(mgr, platform, "clusteroperator"), - Scheme: mgr.GetScheme(), - IsUnsupportedPlatform: !supportedPlatform, + Client: mgr.GetClient(), + ReleaseVersion: util.GetReleaseVersion(), + IsUnsupportedPlatform: !supportedPlatform, }).SetupWithManager(mgr); err != nil { return fmt.Errorf("unable to create clusteroperator controller: %w", err) } - // The ClusterOperatorController MUST run if we were installed, otherwise - // our ClusterOperator will not be reconciled and installation will not - // progress. We don't run any other controllers if the current platform is - // not supported. - if !supportedPlatform { - return nil - } - - providerProfiles, err := loadProviderImages(ctx, mgr, providerImageDir) + // Get container image from own pod spec + containerImage, err := getContainerImage(ctx, mgr.GetAPIReader()) if err != nil { - return err - } - - for _, profile := range providerProfiles { - log.Info("loaded provider profile", "name", profile.Name, "imageRef", profile.ImageRef, "profile", profile.Profile) - } - - if err := (&revision.RevisionController{ - Client: mgr.GetClient(), - ProviderProfiles: providerProfiles, - ReleaseVersion: util.GetReleaseVersion(), - }).SetupWithManager(mgr, operatorConfig.TLSOptions); err != nil { - log.Error(err, "unable to create revision controller", "controller", "RevisionController") - return fmt.Errorf("unable to create revision controller: %w", err) + return fmt.Errorf("unable to get container image: %w", err) } - if err := installer.SetupWithManager(mgr, providerProfiles); err != nil { - return fmt.Errorf("unable to create installer controller: %w", err) + // Setup InstallerDeploymentController (runs on all platforms) + if err := (&installerdeployment.InstallerDeploymentReconciler{ + Client: mgr.GetClient(), + Namespace: *operatorConfig.OperatorNamespace, + ContainerImage: containerImage, + SupportedPlatform: supportedPlatform, + }).SetupWithManager(mgr); err != nil { + return fmt.Errorf("unable to create installerdeployment controller: %w", err) } return nil } -func loadProviderImages(ctx context.Context, mgr ctrl.Manager, providerImageDir string) ([]providerimages.ProviderImageManifests, error) { +// getContainerImage reads the container image from the capi-operator pod spec. +func getContainerImage(ctx context.Context, k8sClient client.Reader) (string, error) { podName := os.Getenv("POD_NAME") - podNamespace := os.Getenv("POD_NAMESPACE") + if podName == "" || podNamespace == "" { - return nil, errPodIdentityNotSet + return "", errPodIdentityNotSet } var pod corev1.Pod - if err := mgr.GetAPIReader().Get(ctx, types.NamespacedName{Name: podName, Namespace: podNamespace}, &pod); err != nil { - return nil, fmt.Errorf("unable to get pod %s/%s: %w", podNamespace, podName, err) + if err := k8sClient.Get(ctx, types.NamespacedName{Name: podName, Namespace: podNamespace}, &pod); err != nil { + return "", fmt.Errorf("unable to get pod %s/%s: %w", podNamespace, podName, err) } - imageRefMap, err := providerimages.BuildImageRefMap(pod.Spec, managerName) - if err != nil { - return nil, fmt.Errorf("unable to build image ref map from pod spec: %w", err) - } - - log := ctrl.LoggerFrom(ctx) - - providerProfiles, err := providerimages.ScanProviderImages(log, providerImageDir, imageRefMap) - if err != nil { - return nil, fmt.Errorf("unable to scan provider images: %w", err) + // Find the capi-operator container + for _, container := range pod.Spec.Containers { + if container.Name == managerName { + return container.Image, nil + } } - return providerProfiles, nil + return "", fmt.Errorf("%s: %w", managerName, errContainerNotInPod) } diff --git a/go.mod b/go.mod index a550e009e9..700f520231 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ tool ( github.com/openshift/api/machine/v1beta1/zz_generated.crd-manifests github.com/openshift/api/operator/v1/zz_generated.crd-manifests github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests + golang.org/x/tools/cmd/stringer sigs.k8s.io/controller-runtime/tools/setup-envtest ) diff --git a/manifests/0000_30_cluster-api-installer_05_deployment.yaml b/manifests/0000_30_cluster-api-installer_05_deployment.yaml deleted file mode 100644 index 68eb30f4da..0000000000 --- a/manifests/0000_30_cluster-api-installer_05_deployment.yaml +++ /dev/null @@ -1,160 +0,0 @@ ---- -apiVersion: apps/v1 -kind: Deployment -metadata: - name: capi-operator - namespace: openshift-cluster-api-operator - annotations: - config.openshift.io/inject-proxy: capi-operator - include.release.openshift.io/self-managed-high-availability: "true" - include.release.openshift.io/single-node-developer: "true" - exclude.release.openshift.io/internal-openshift-hosted: "true" - release.openshift.io/feature-gate: "ClusterAPIMachineManagement" - labels: - k8s-app: capi-operator -spec: - selector: - matchLabels: - k8s-app: capi-operator - replicas: 1 - template: - metadata: - annotations: - target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' - openshift.io/required-scc: restricted-v2 - labels: - k8s-app: capi-operator - spec: - serviceAccountName: capi-operator - containers: - - name: capi-operator - image: registry.ci.openshift.org/openshift:cluster-capi-operator - command: - - /capi-operator - args: - - --diagnostics-address=:8443 - env: - - name: RELEASE_VERSION - value: "0.0.1-snapshot" - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - ports: - - containerPort: 9440 - name: health - protocol: TCP - - containerPort: 8443 - name: diagnostics - protocol: TCP - resources: - requests: - cpu: 10m - memory: 50Mi - terminationMessagePolicy: FallbackToLogsOnError - volumeMounts: - - name: metrics-cert - mountPath: /tmp/k8s-metrics-server/serving-certs - readOnly: true - - name: provider-aws - mountPath: /var/lib/provider-images/aws-cluster-api-controllers - readOnly: true - - name: provider-azure - mountPath: /var/lib/provider-images/azure-cluster-api-controllers - readOnly: true - - name: provider-baremetal - mountPath: /var/lib/provider-images/baremetal-cluster-api-controllers - readOnly: true - - name: provider-cluster-capi-controllers - mountPath: /var/lib/provider-images/cluster-capi-controllers - readOnly: true - - name: provider-cluster-capi-operator - mountPath: /var/lib/provider-images/cluster-capi-operator - readOnly: true - - name: provider-gcp - mountPath: /var/lib/provider-images/gcp-cluster-api-controllers - readOnly: true - - name: provider-ibmcloud - mountPath: /var/lib/provider-images/ibmcloud-cluster-api-controllers - readOnly: true - - name: provider-openstack - mountPath: /var/lib/provider-images/openstack-cluster-api-controllers - readOnly: true - - name: provider-openstack-resource-controller - mountPath: /var/lib/provider-images/openstack-resource-controller - readOnly: true - - name: provider-vsphere - mountPath: /var/lib/provider-images/vsphere-cluster-api-controllers - readOnly: true - livenessProbe: - httpGet: - path: /healthz - port: 9440 - initialDelaySeconds: 15 - periodSeconds: 20 - readinessProbe: - httpGet: - path: /readyz - port: 9440 - initialDelaySeconds: 5 - periodSeconds: 10 - nodeSelector: - node-role.kubernetes.io/control-plane: "" - priorityClassName: system-node-critical - restartPolicy: Always - tolerations: - - key: "node-role.kubernetes.io/master" - operator: "Exists" - effect: "NoSchedule" - - key: "node-role.kubernetes.io/control-plane" - operator: "Exists" - effect: "NoSchedule" - volumes: - - name: metrics-cert - secret: - defaultMode: 420 - secretName: capi-operator-metrics-tls - - name: provider-aws - image: - reference: registry.ci.openshift.org/openshift:aws-cluster-api-controllers - pullPolicy: IfNotPresent - - name: provider-azure - image: - reference: registry.ci.openshift.org/openshift:azure-cluster-api-controllers - pullPolicy: IfNotPresent - - name: provider-baremetal - image: - reference: registry.ci.openshift.org/openshift:baremetal-cluster-api-controllers - pullPolicy: IfNotPresent - - name: provider-cluster-capi-controllers - image: - reference: registry.ci.openshift.org/openshift:cluster-capi-controllers - pullPolicy: IfNotPresent - - name: provider-cluster-capi-operator - image: - reference: registry.ci.openshift.org/openshift:cluster-capi-operator - pullPolicy: IfNotPresent - - name: provider-gcp - image: - reference: registry.ci.openshift.org/openshift:gcp-cluster-api-controllers - pullPolicy: IfNotPresent - - name: provider-ibmcloud - image: - reference: registry.ci.openshift.org/openshift:ibmcloud-cluster-api-controllers - pullPolicy: IfNotPresent - - name: provider-openstack - image: - reference: registry.ci.openshift.org/openshift:openstack-cluster-api-controllers - pullPolicy: IfNotPresent - - name: provider-openstack-resource-controller - image: - reference: registry.ci.openshift.org/openshift:openstack-resource-controller - pullPolicy: IfNotPresent - - name: provider-vsphere - image: - reference: registry.ci.openshift.org/openshift:vsphere-cluster-api-controllers - pullPolicy: IfNotPresent diff --git a/manifests/0000_30_cluster-api-installer_00_namespace.yaml b/manifests/0000_30_cluster-api-operator_00_namespace.yaml similarity index 100% rename from manifests/0000_30_cluster-api-installer_00_namespace.yaml rename to manifests/0000_30_cluster-api-operator_00_namespace.yaml diff --git a/manifests/0000_30_cluster-api-installer_00_tombstones.yaml b/manifests/0000_30_cluster-api-operator_00_tombstones.yaml similarity index 100% rename from manifests/0000_30_cluster-api-installer_00_tombstones.yaml rename to manifests/0000_30_cluster-api-operator_00_tombstones.yaml diff --git a/manifests/0000_30_cluster-api-installer_01_metrics-service.yaml b/manifests/0000_30_cluster-api-operator_01_metrics-service.yaml similarity index 100% rename from manifests/0000_30_cluster-api-installer_01_metrics-service.yaml rename to manifests/0000_30_cluster-api-operator_01_metrics-service.yaml diff --git a/manifests/0000_30_cluster-api-installer_01_serviceaccount.yaml b/manifests/0000_30_cluster-api-operator_01_serviceaccount.yaml similarity index 100% rename from manifests/0000_30_cluster-api-installer_01_serviceaccount.yaml rename to manifests/0000_30_cluster-api-operator_01_serviceaccount.yaml diff --git a/manifests/0000_30_cluster-api-operator_02_capi-installer-metrics-service.yaml b/manifests/0000_30_cluster-api-operator_02_capi-installer-metrics-service.yaml new file mode 100644 index 0000000000..bcd9f2c60b --- /dev/null +++ b/manifests/0000_30_cluster-api-operator_02_capi-installer-metrics-service.yaml @@ -0,0 +1,21 @@ +apiVersion: v1 +kind: Service +metadata: + annotations: + exclude.release.openshift.io/internal-openshift-hosted: "true" + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + release.openshift.io/feature-gate: "ClusterAPIMachineManagement" + service.beta.openshift.io/serving-cert-secret-name: capi-installer-metrics-tls + name: capi-installer-metrics + namespace: openshift-cluster-api-operator +spec: + ports: + - name: diagnostics + port: 8443 + targetPort: diagnostics + selector: + k8s-app: capi-installer + type: ClusterIP + clusterIP: None + sessionAffinity: None diff --git a/manifests/0000_30_cluster-api-operator_02_capi-installer-serviceaccount.yaml b/manifests/0000_30_cluster-api-operator_02_capi-installer-serviceaccount.yaml new file mode 100644 index 0000000000..ae0deabb92 --- /dev/null +++ b/manifests/0000_30_cluster-api-operator_02_capi-installer-serviceaccount.yaml @@ -0,0 +1,11 @@ +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + namespace: openshift-cluster-api-operator + name: capi-installer + annotations: + exclude.release.openshift.io/internal-openshift-hosted: "true" + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + release.openshift.io/feature-gate: "ClusterAPIMachineManagement" diff --git a/manifests/0000_30_cluster-api-operator_02_capi-installer-servicemonitor.yaml b/manifests/0000_30_cluster-api-operator_02_capi-installer-servicemonitor.yaml new file mode 100644 index 0000000000..d26c64399c --- /dev/null +++ b/manifests/0000_30_cluster-api-operator_02_capi-installer-servicemonitor.yaml @@ -0,0 +1,23 @@ +apiVersion: monitoring.coreos.com/v1 +kind: ServiceMonitor +metadata: + annotations: + exclude.release.openshift.io/internal-openshift-hosted: "true" + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + release.openshift.io/feature-gate: "ClusterAPIMachineManagement" + name: capi-installer + namespace: openshift-cluster-api-operator +spec: + endpoints: + - bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token + port: diagnostics + scheme: https + tlsConfig: + caFile: /etc/prometheus/configmaps/serving-certs-ca-bundle/service-ca.crt + certFile: /etc/prometheus/secrets/metrics-client-certs/tls.crt + keyFile: /etc/prometheus/secrets/metrics-client-certs/tls.key + serverName: capi-installer-metrics.openshift-cluster-api-operator.svc + selector: + matchLabels: + k8s-app: capi-installer diff --git a/manifests/0000_30_cluster-api-installer_02_clusterrole.yaml b/manifests/0000_30_cluster-api-operator_03_clusterrole.yaml similarity index 100% rename from manifests/0000_30_cluster-api-installer_02_clusterrole.yaml rename to manifests/0000_30_cluster-api-operator_03_clusterrole.yaml diff --git a/manifests/0000_30_cluster-api-operator_04_capi-installer-clusterrolebinding.yaml b/manifests/0000_30_cluster-api-operator_04_capi-installer-clusterrolebinding.yaml new file mode 100644 index 0000000000..64eb10129c --- /dev/null +++ b/manifests/0000_30_cluster-api-operator_04_capi-installer-clusterrolebinding.yaml @@ -0,0 +1,18 @@ +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: openshift-capi-installer + annotations: + exclude.release.openshift.io/internal-openshift-hosted: "true" + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + release.openshift.io/feature-gate: "ClusterAPIMachineManagement" +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: openshift-capi-operator +subjects: +- kind: ServiceAccount + name: capi-installer + namespace: openshift-cluster-api-operator diff --git a/manifests/0000_30_cluster-api-installer_03_clusterrolebinding.yaml b/manifests/0000_30_cluster-api-operator_04_clusterrolebinding.yaml similarity index 100% rename from manifests/0000_30_cluster-api-installer_03_clusterrolebinding.yaml rename to manifests/0000_30_cluster-api-operator_04_clusterrolebinding.yaml diff --git a/manifests/0000_30_cluster-api-operator_05_allow-egress-operators.yaml b/manifests/0000_30_cluster-api-operator_05_allow-egress-operators.yaml new file mode 100644 index 0000000000..4d50cfbc5b --- /dev/null +++ b/manifests/0000_30_cluster-api-operator_05_allow-egress-operators.yaml @@ -0,0 +1,27 @@ +# This NetworkPolicy allows egress traffic required for the CAPI operator +# deployments in the openshift-cluster-api-operator namespace. +# The operator and installer need broad internet access for cluster management +# operations, cloud provider API calls, and communication with various services. +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + annotations: + exclude.release.openshift.io/internal-openshift-hosted: "true" + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + release.openshift.io/feature-gate: "ClusterAPIMachineManagement" + name: allow-egress-operators + namespace: openshift-cluster-api-operator +spec: + egress: + # Allow all egress traffic - operator needs broad access + - {} # Empty rule allows all egress + podSelector: + matchExpressions: + - key: k8s-app + operator: In + values: + - capi-operator + - capi-installer + policyTypes: + - Egress diff --git a/manifests/0000_30_cluster-api-operator_05_provider-images-configmap.yaml b/manifests/0000_30_cluster-api-operator_05_provider-images-configmap.yaml new file mode 100644 index 0000000000..cf83d63ceb --- /dev/null +++ b/manifests/0000_30_cluster-api-operator_05_provider-images-configmap.yaml @@ -0,0 +1,22 @@ +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: capi-installer-images + namespace: openshift-cluster-api-operator + annotations: + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + exclude.release.openshift.io/internal-openshift-hosted: "true" + release.openshift.io/feature-gate: "ClusterAPIMachineManagement" +data: + aws-cluster-api-controllers: registry.ci.openshift.org/openshift:aws-cluster-api-controllers + azure-cluster-api-controllers: registry.ci.openshift.org/openshift:azure-cluster-api-controllers + baremetal-cluster-api-controllers: registry.ci.openshift.org/openshift:baremetal-cluster-api-controllers + cluster-capi-controllers: registry.ci.openshift.org/openshift:cluster-capi-controllers + cluster-capi-operator: registry.ci.openshift.org/openshift:cluster-capi-operator + gcp-cluster-api-controllers: registry.ci.openshift.org/openshift:gcp-cluster-api-controllers + ibmcloud-cluster-api-controllers: registry.ci.openshift.org/openshift:ibmcloud-cluster-api-controllers + openstack-cluster-api-controllers: registry.ci.openshift.org/openshift:openstack-cluster-api-controllers + openstack-resource-controller: registry.ci.openshift.org/openshift:openstack-resource-controller + vsphere-cluster-api-controllers: registry.ci.openshift.org/openshift:vsphere-cluster-api-controllers diff --git a/manifests/0000_30_cluster-api-operator_06_deployment.yaml b/manifests/0000_30_cluster-api-operator_06_deployment.yaml new file mode 100644 index 0000000000..1a119d6f43 --- /dev/null +++ b/manifests/0000_30_cluster-api-operator_06_deployment.yaml @@ -0,0 +1,89 @@ +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: capi-operator + namespace: openshift-cluster-api-operator + annotations: + config.openshift.io/inject-proxy: capi-operator + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + exclude.release.openshift.io/internal-openshift-hosted: "true" + release.openshift.io/feature-gate: "ClusterAPIMachineManagement" + labels: + k8s-app: capi-operator +spec: + selector: + matchLabels: + k8s-app: capi-operator + replicas: 1 + template: + metadata: + annotations: + target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' + openshift.io/required-scc: restricted-v2 + labels: + k8s-app: capi-operator + spec: + serviceAccountName: capi-operator + containers: + - name: capi-operator + image: registry.ci.openshift.org/openshift:cluster-capi-operator + command: + - /capi-operator + args: + - --diagnostics-address=:8443 + env: + - name: RELEASE_VERSION + value: "0.0.1-snapshot" + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + ports: + - containerPort: 9440 + name: health + protocol: TCP + - containerPort: 8443 + name: diagnostics + protocol: TCP + resources: + requests: + cpu: 10m + memory: 50Mi + terminationMessagePolicy: FallbackToLogsOnError + volumeMounts: + - name: metrics-cert + mountPath: /tmp/k8s-metrics-server/serving-certs + readOnly: true + livenessProbe: + httpGet: + path: /healthz + port: 9440 + initialDelaySeconds: 15 + periodSeconds: 20 + readinessProbe: + httpGet: + path: /readyz + port: 9440 + initialDelaySeconds: 5 + periodSeconds: 10 + nodeSelector: + node-role.kubernetes.io/control-plane: "" + restartPolicy: Always + tolerations: + - key: "node-role.kubernetes.io/master" + operator: "Exists" + effect: "NoSchedule" + - key: "node-role.kubernetes.io/control-plane" + operator: "Exists" + effect: "NoSchedule" + volumes: + - name: metrics-cert + secret: + defaultMode: 420 + secretName: capi-operator-metrics-tls diff --git a/manifests/0000_30_cluster-api-installer_06_clusterapi.yaml b/manifests/0000_30_cluster-api-operator_07_clusterapi.yaml similarity index 100% rename from manifests/0000_30_cluster-api-installer_06_clusterapi.yaml rename to manifests/0000_30_cluster-api-operator_07_clusterapi.yaml diff --git a/manifests/0000_30_cluster-api_12_clusteroperator.yaml b/manifests/0000_30_cluster-api-operator_08_clusteroperator.yaml similarity index 100% rename from manifests/0000_30_cluster-api_12_clusteroperator.yaml rename to manifests/0000_30_cluster-api-operator_08_clusteroperator.yaml diff --git a/manifests/0000_30_cluster-api_13_allow-ingress-to-metrics-controllers.yaml b/manifests/0000_30_cluster-api_11_allow-ingress-to-metrics-controllers.yaml similarity index 100% rename from manifests/0000_30_cluster-api_13_allow-ingress-to-metrics-controllers.yaml rename to manifests/0000_30_cluster-api_11_allow-ingress-to-metrics-controllers.yaml diff --git a/manifests/0000_30_cluster-api_14_allow-ingress-to-metrics-operators.yaml b/manifests/0000_30_cluster-api_12_allow-ingress-to-metrics-operators.yaml similarity index 99% rename from manifests/0000_30_cluster-api_14_allow-ingress-to-metrics-operators.yaml rename to manifests/0000_30_cluster-api_12_allow-ingress-to-metrics-operators.yaml index 1e60d183df..fc48999dd2 100644 --- a/manifests/0000_30_cluster-api_14_allow-ingress-to-metrics-operators.yaml +++ b/manifests/0000_30_cluster-api_12_allow-ingress-to-metrics-operators.yaml @@ -37,6 +37,7 @@ spec: operator: In values: - capi-operator + - capi-installer policyTypes: - Ingress --- diff --git a/manifests/0000_30_cluster-api_15_allow-egress-controllers.yaml b/manifests/0000_30_cluster-api_13_allow-egress-controllers.yaml similarity index 100% rename from manifests/0000_30_cluster-api_15_allow-egress-controllers.yaml rename to manifests/0000_30_cluster-api_13_allow-egress-controllers.yaml diff --git a/manifests/0000_30_cluster-api_14_allow-egress-operators.yaml b/manifests/0000_30_cluster-api_14_allow-egress-operators.yaml new file mode 100644 index 0000000000..ac6c77e784 --- /dev/null +++ b/manifests/0000_30_cluster-api_14_allow-egress-operators.yaml @@ -0,0 +1,29 @@ +# This NetworkPolicy allows egress traffic required for the CAPI controller +# deployments in the openshift-cluster-api namespace. +# The controllers need broad internet access for cluster management operations, +# cloud provider API calls, and communication with various services. +# +# This approach is more practical than overly granular rules since the operator +# needs broad access to function properly in various environments. +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + annotations: + exclude.release.openshift.io/internal-openshift-hosted: "true" + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + release.openshift.io/feature-gate: "ClusterAPIMachineManagement" + name: allow-egress-operators + namespace: openshift-cluster-api +spec: + egress: + # Allow all egress traffic - operator needs broad access + - {} # Empty rule allows all egress + podSelector: + matchExpressions: + - key: k8s-app + operator: In + values: + - capi-controllers + policyTypes: + - Egress diff --git a/manifests/0000_30_cluster-api_17_default-deny.yaml b/manifests/0000_30_cluster-api_15_default-deny.yaml similarity index 100% rename from manifests/0000_30_cluster-api_17_default-deny.yaml rename to manifests/0000_30_cluster-api_15_default-deny.yaml diff --git a/manifests/0000_30_cluster-api_16_allow-egress-operators.yaml b/manifests/0000_30_cluster-api_16_allow-egress-operators.yaml deleted file mode 100644 index 7f3f3863f9..0000000000 --- a/manifests/0000_30_cluster-api_16_allow-egress-operators.yaml +++ /dev/null @@ -1,58 +0,0 @@ -# These NetworkPolicies allows egress traffic required for the CAPI operator -# deployments. -# The operator needs broad internet access for cluster management operations, -# cloud provider API calls, and communication with various services. -# -# This policy allows all egress traffic from the capi-controllers pod, which is -# necessary because the operator needs to communicate with: -# - Kubernetes API server for cluster management operations -# - Cloud provider APIs for infrastructure management -# - Container registries and other external services -# -# This approach is more practical than overly granular rules since the operator -# needs broad access to function properly in various environments. -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - annotations: - exclude.release.openshift.io/internal-openshift-hosted: "true" - include.release.openshift.io/self-managed-high-availability: "true" - include.release.openshift.io/single-node-developer: "true" - release.openshift.io/feature-gate: "ClusterAPIMachineManagement" - name: allow-egress-operators - namespace: openshift-cluster-api -spec: - egress: - # Allow all egress traffic - operator needs broad access - - {} # Empty rule allows all egress - podSelector: - matchExpressions: - - key: k8s-app - operator: In - values: - - capi-controllers - policyTypes: - - Egress ---- -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - annotations: - exclude.release.openshift.io/internal-openshift-hosted: "true" - include.release.openshift.io/self-managed-high-availability: "true" - include.release.openshift.io/single-node-developer: "true" - release.openshift.io/feature-gate: "ClusterAPIMachineManagement" - name: allow-egress-operators - namespace: openshift-cluster-api-operator -spec: - egress: - # Allow all egress traffic - operator needs broad access - - {} # Empty rule allows all egress - podSelector: - matchExpressions: - - key: k8s-app - operator: In - values: - - capi-operator - policyTypes: - - Egress diff --git a/manifests/0000_30_cluster-api_18_allow-ingress-to-webhook.yaml b/manifests/0000_30_cluster-api_16_allow-ingress-to-webhook.yaml similarity index 100% rename from manifests/0000_30_cluster-api_18_allow-ingress-to-webhook.yaml rename to manifests/0000_30_cluster-api_16_allow-ingress-to-webhook.yaml diff --git a/manifests/0000_30_cluster-api_11_deployment.yaml b/manifests/0000_30_cluster-api_17_deployment.yaml similarity index 100% rename from manifests/0000_30_cluster-api_11_deployment.yaml rename to manifests/0000_30_cluster-api_17_deployment.yaml diff --git a/pkg/controllers/clusteroperator/clusteroperator_controller.go b/pkg/controllers/clusteroperator/clusteroperator_controller.go index ab6b79b74b..f32633323c 100644 --- a/pkg/controllers/clusteroperator/clusteroperator_controller.go +++ b/pkg/controllers/clusteroperator/clusteroperator_controller.go @@ -19,15 +19,20 @@ package clusteroperator import ( "context" "fmt" - - "k8s.io/apimachinery/pkg/runtime" + "slices" + "strings" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" configv1 "github.com/openshift/api/config/v1" + configv1apply "github.com/openshift/client-go/config/applyconfigurations/config/v1" "github.com/openshift/cluster-capi-operator/pkg/controllers" + "github.com/openshift/cluster-capi-operator/pkg/controllers/installer" + "github.com/openshift/cluster-capi-operator/pkg/controllers/revision" "github.com/openshift/cluster-capi-operator/pkg/operatorstatus" + "github.com/openshift/cluster-capi-operator/pkg/util" ) const ( @@ -35,42 +40,229 @@ const ( controllerName = "ClusterOperatorController" ) -// ClusterOperatorController watches and keeps the cluster-api ClusterObject up to date. +// ClusterOperatorController watches the cluster-api ClusterOperator and +// aggregates per-controller sub-conditions into top-level conditions. type ClusterOperatorController struct { - operatorstatus.ClusterOperatorStatusClient - Scheme *runtime.Scheme + client.Client + ReleaseVersion string IsUnsupportedPlatform bool } // Reconcile reconciles the cluster-api ClusterOperator object. -func (r *ClusterOperatorController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +func (r *ClusterOperatorController) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result, error) { log := ctrl.LoggerFrom(ctx).WithName(controllerName) - log.Info(fmt.Sprintf("Reconciling %q ClusterObject", controllers.ClusterOperatorName)) - message := func() string { - if r.IsUnsupportedPlatform { - return capiUnsupportedPlatformMsg - } + co := &configv1.ClusterOperator{} + if err := r.Get(ctx, client.ObjectKey{Name: controllers.ClusterOperatorName}, co); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to get ClusterOperator: %w", err) + } + + log.Info("Reconciling ClusterOperator aggregation") - return "" - }() + var conditions []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration - // TODO: wrap this into status aggregation logic to get these conditions to - // represent the meaningful aggregation of all other controller statuses. - // We should also only update the version after the aggregator confirms - // all controllers have succeeded in the new version. - if err := r.SetStatusAvailable(ctx, message, operatorstatus.WithVersions(r.OperandVersions())); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to set conditions for %q ClusterObject: %w", controllers.ClusterOperatorName, err) + if r.IsUnsupportedPlatform { + conditions = r.unsupportedPlatformStatus() + } else { + conditions = r.aggregatedStatus(co.Status.Conditions) + } + + // Merge new conditions with existing conditions and patch if changes are required. + conditionsChanged := operatorstatus.MergeConditions(conditions, co.Status.Conditions) + versionChanged := r.IsUnsupportedPlatform && + currentOperatorVersion(co.Status.Versions, operatorstatus.OperatorVersionKey) != r.ReleaseVersion + + if conditionsChanged || versionChanged { + if err := r.writeStatus(ctx, co, conditions); err != nil { + return ctrl.Result{}, err + } } return ctrl.Result{}, nil } +// currentOperatorVersion returns the version string for the given key in the +// ClusterOperator's status versions list, or an empty string if not found. +func currentOperatorVersion(versions []configv1.OperandVersion, name string) string { + for i := range versions { + if versions[i].Name == name { + return versions[i].Version + } + } + + return "" +} + +func (r *ClusterOperatorController) writeStatus(ctx context.Context, co *configv1.ClusterOperator, conditions []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration) error { + applyConfig := configv1apply.ClusterOperator(controllers.ClusterOperatorName). + WithUID(co.UID). + WithStatus(configv1apply.ClusterOperatorStatus(). + WithConditions(conditions...), + ) + + // We don't run the revision controller on unsupported platforms, so we must + // write the release version here. + if r.IsUnsupportedPlatform { + applyConfig.Status = applyConfig.Status.WithVersions( + configv1apply.OperandVersion(). + WithName(operatorstatus.OperatorVersionKey). + WithVersion(r.ReleaseVersion)) + } + + if err := r.Status().Patch(ctx, co, util.ApplyConfigPatch(applyConfig), + operatorstatus.CAPIFieldOwner(controllerName), client.ForceOwnership); err != nil { + return fmt.Errorf("failed to write ClusterOperator status: %w", err) + } + + return nil +} + +// unsupportedPlatformStatus sets a fixed status with Available=true, +// Progressing=false, Degraded=false, Upgradeable=true when running on an +// unsupported platform. +func (r *ClusterOperatorController) unsupportedPlatformStatus() []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration { + return []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + condition(configv1.OperatorAvailable, configv1.ConditionTrue, operatorstatus.ReasonAsExpected, capiUnsupportedPlatformMsg), + condition(configv1.OperatorProgressing, configv1.ConditionFalse, operatorstatus.ReasonAsExpected, ""), + condition(configv1.OperatorDegraded, configv1.ConditionFalse, operatorstatus.ReasonAsExpected, ""), + condition(configv1.OperatorUpgradeable, configv1.ConditionTrue, operatorstatus.ReasonAsExpected, ""), + } +} + +type subcontrollerStatus struct { + controller operatorstatus.ControllerResultGenerator + available, progressing subcontrollerCondition +} + +type subcontrollerCondition struct { + status configv1.ConditionStatus + reason operatorstatus.Reason + message string +} + +func getSubcontrollerCondition(conditions []configv1.ClusterOperatorStatusCondition, condType configv1.ClusterStatusConditionType) subcontrollerCondition { + for i := range conditions { + if conditions[i].Type == condType { + return subcontrollerCondition{ + status: conditions[i].Status, + reason: operatorstatus.ReasonFromString(conditions[i].Reason), + message: conditions[i].Message, + } + } + } + + return subcontrollerCondition{ + status: configv1.ConditionUnknown, + reason: operatorstatus.ReasonUninitialized, + message: "initializing", + } +} + +func (r *ClusterOperatorController) aggregatedStatus(currentConditions []configv1.ClusterOperatorStatusCondition) []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration { + newConditions := []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + // The capi operator does not yet set the degraded condition. This will + // be added by automatically flagging a Progressing condition which + // lasts longer than some duration. + condition(configv1.OperatorDegraded, configv1.ConditionFalse, operatorstatus.ReasonAsExpected, ""), + + // Nothing the capi operator currently does prevents upgradeability. + // This will be added when CRD compatibility is integrated with the + // installer and revision controllers. + condition(configv1.OperatorUpgradeable, configv1.ConditionTrue, operatorstatus.ReasonAsExpected, ""), + } + + // Sub-controllers whose Progressing and Degraded conditions are aggregated + subControllers := []operatorstatus.ControllerResultGenerator{ + installer.ResultGenerator, + revision.ResultGenerator, + // TBD as they are migrated: + // - corecluster + // - infracluster + // - secretsync + // - kubeconfig + } + + subcontrollerStatuses := util.SliceMap(subControllers, func(subController operatorstatus.ControllerResultGenerator) subcontrollerStatus { + availableType := subController.SubConditionType(operatorstatus.ConditionAvailableSuffix) + progressingType := subController.SubConditionType(operatorstatus.ConditionProgressingSuffix) + + return subcontrollerStatus{ + controller: subController, + available: getSubcontrollerCondition(currentConditions, availableType), + progressing: getSubcontrollerCondition(currentConditions, progressingType), + } + }) + + isProgressing := slices.IndexFunc(subcontrollerStatuses, func(status subcontrollerStatus) bool { + return status.progressing.status == configv1.ConditionTrue || status.progressing.status == configv1.ConditionUnknown + }) >= 0 + progressingReason, progressingMessage := aggregateReasonAndMessage(subcontrollerStatuses, func(s subcontrollerStatus) subcontrollerCondition { + return s.progressing + }) + + switch { + case isProgressing: + newConditions = append(newConditions, condition(configv1.OperatorProgressing, configv1.ConditionTrue, progressingReason, progressingMessage)) + case progressingReason > operatorstatus.ReasonAsExpected: + newConditions = append(newConditions, condition(configv1.OperatorProgressing, configv1.ConditionFalse, progressingReason, progressingMessage)) + default: + newConditions = append(newConditions, condition(configv1.OperatorProgressing, configv1.ConditionFalse, operatorstatus.ReasonAsExpected, "")) + } + + notAvailable := slices.IndexFunc(subcontrollerStatuses, func(status subcontrollerStatus) bool { + return status.available.status != configv1.ConditionTrue + }) >= 0 + availableReason, availableMessage := aggregateReasonAndMessage(subcontrollerStatuses, func(s subcontrollerStatus) subcontrollerCondition { + return s.available + }) + + if notAvailable { + newConditions = append(newConditions, condition(configv1.OperatorAvailable, configv1.ConditionFalse, availableReason, availableMessage)) + } else { + newConditions = append(newConditions, condition(configv1.OperatorAvailable, configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Cluster API Operator is available")) + } + + return newConditions +} + +func aggregateReasonAndMessage(statuses []subcontrollerStatus, extract func(subcontrollerStatus) subcontrollerCondition) (operatorstatus.Reason, string) { + var maxReason operatorstatus.Reason + + var parts []string + + for _, s := range statuses { + cond := extract(s) + if cond.reason <= operatorstatus.ReasonAsExpected { + continue + } + + if cond.reason > maxReason { + maxReason = cond.reason + } + + if cond.message != "" { + parts = append(parts, fmt.Sprintf("%s: %s", s.controller, cond.message)) + } else { + parts = append(parts, string(s.controller)) + } + } + + return maxReason, strings.Join(parts, "; ") +} + +func condition(condType configv1.ClusterStatusConditionType, status configv1.ConditionStatus, reason operatorstatus.Reason, message string) *configv1apply.ClusterOperatorStatusConditionApplyConfiguration { + return configv1apply.ClusterOperatorStatusCondition(). + WithType(condType). + WithStatus(status). + WithReason(reason.String()). + WithMessage(message) +} + // SetupWithManager sets up the controller with the Manager. func (r *ClusterOperatorController) SetupWithManager(mgr ctrl.Manager) error { if err := ctrl.NewControllerManagedBy(mgr). Named(controllerName). - For(&configv1.ClusterOperator{}, builder.WithPredicates(operatorstatus.ClusterOperatorOnceOnly())). + For(&configv1.ClusterOperator{}, builder.WithPredicates(operatorstatus.ClusterOperatorStatusChanged())). Complete(r); err != nil { return fmt.Errorf("failed to create controller: %w", err) } diff --git a/pkg/controllers/clusteroperator/clusteroperator_controller_test.go b/pkg/controllers/clusteroperator/clusteroperator_controller_test.go index bbedcc2550..bbe35611f1 100644 --- a/pkg/controllers/clusteroperator/clusteroperator_controller_test.go +++ b/pkg/controllers/clusteroperator/clusteroperator_controller_test.go @@ -18,8 +18,6 @@ package clusteroperator import ( "context" - "fmt" - "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -28,18 +26,21 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/config" - "sigs.k8s.io/controller-runtime/pkg/envtest/komega" "sigs.k8s.io/controller-runtime/pkg/metrics/server" configv1 "github.com/openshift/api/config/v1" + configv1apply "github.com/openshift/client-go/config/applyconfigurations/config/v1" "github.com/openshift/cluster-api-actuator-pkg/testutils" configv1resourcebuilder "github.com/openshift/cluster-api-actuator-pkg/testutils/resourcebuilder/config/v1" - corev1resourcebuilder "github.com/openshift/cluster-api-actuator-pkg/testutils/resourcebuilder/core/v1" "github.com/openshift/cluster-capi-operator/pkg/controllers" "github.com/openshift/cluster-capi-operator/pkg/operatorstatus" + "github.com/openshift/cluster-capi-operator/pkg/test" + "github.com/openshift/cluster-capi-operator/pkg/util" ) -const desiredOperatorReleaseVersion = "this-is-the-desired-release-version" +const ( + desiredOperatorReleaseVersion = "this-is-the-desired-release-version" +) var ( mgrCancel context.CancelFunc @@ -47,127 +48,399 @@ var ( ) var _ = Describe("ClusterOperator controller", func() { - ctx := context.Background() - - var ( - capiClusterOperator *configv1.ClusterOperator - testNamespaceName string - ) + Context("with a supported platform", func() { + var capiClusterOperator *configv1.ClusterOperator - BeforeEach(func() { - By("Creating the cluster-api ClusterOperator") + BeforeEach(func(ctx context.Context) { + mgrCancel, mgrDone = startManager(false) - capiClusterOperator = &configv1.ClusterOperator{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cluster-api", + DeferCleanup(stopManager) + + By("Creating the cluster-api ClusterOperator with a previous release version", func() { + capiClusterOperator = &configv1.ClusterOperator{ + ObjectMeta: metav1.ObjectMeta{ + Name: controllers.ClusterOperatorName, + }, + } + Expect(cl.Create(ctx, capiClusterOperator)).To(Succeed()) + DeferCleanup(func(ctx context.Context) { + testutils.CleanupResources(Default, ctx, testEnv.Config, cl, "", &configv1.ClusterOperator{}) + }) + Expect(cl.Status().Update(ctx, capiClusterOperator)).To(Succeed()) + }) + }, defaultNodeTimeout) + + DescribeTable("rollup aggregation", + func(ctx context.Context, subConditions []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration, + expectedAvailable, expectedProgressing *test.ConditionMatcher) { + if len(subConditions) > 0 { + patchSubConditions(ctx, capiClusterOperator, subConditions...) + } + + co := kWithCtx(ctx).Object(configv1resourcebuilder.ClusterOperator().WithName(controllers.ClusterOperatorName).Build()) + + Eventually(co). + WithContext(ctx). + WithTimeout(defaultEventuallyTimeout). + Should(SatisfyAll( + HaveField("Status.Conditions", SatisfyAll( + expectedAvailable, + expectedProgressing, + test.HaveCondition(configv1.OperatorDegraded).WithStatus(configv1.ConditionFalse), + test.HaveCondition(configv1.OperatorUpgradeable).WithStatus(configv1.ConditionTrue), + )), + )) }, - } - Expect(cl.Create(ctx, capiClusterOperator)).To(Succeed(), "should be able to create the 'cluster-api' ClusterOperator object") - - By("Creating the testing namespace") - - namespace := corev1resourcebuilder.Namespace().WithGenerateName("test-capi-corecluster-").Build() - Expect(cl.Create(ctx, namespace)).To(Succeed()) - testNamespaceName = namespace.Name + Entry("when all sub-controllers report success", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + subCondition("InstallerControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("InstallerControllerProgressing", configv1.ConditionFalse, operatorstatus.ReasonAsExpected, "Success"), + subCondition("RevisionControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("RevisionControllerProgressing", configv1.ConditionFalse, operatorstatus.ReasonAsExpected, "Success"), + }, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonAsExpected), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionFalse). + WithReason(operatorstatus.ReasonAsExpected), + defaultNodeTimeout), + Entry("when installer controller is progressing but was previously available", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + subCondition("InstallerControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("InstallerControllerProgressing", configv1.ConditionTrue, operatorstatus.ReasonProgressing, "Installing components"), + subCondition("RevisionControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("RevisionControllerProgressing", configv1.ConditionFalse, operatorstatus.ReasonAsExpected, "Success"), + }, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonAsExpected), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonProgressing). + WithMessage(ContainSubstring("InstallerController")), + defaultNodeTimeout), + Entry("when revision controller is progressing but was previously available", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + subCondition("InstallerControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("InstallerControllerProgressing", configv1.ConditionFalse, operatorstatus.ReasonAsExpected, "Success"), + subCondition("RevisionControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("RevisionControllerProgressing", configv1.ConditionTrue, operatorstatus.ReasonProgressing, "Updating revisions"), + }, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonAsExpected), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonProgressing). + WithMessage(ContainSubstring("RevisionController")), + defaultNodeTimeout), + Entry("when both controllers are progressing but were previously available", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + subCondition("InstallerControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("InstallerControllerProgressing", configv1.ConditionTrue, operatorstatus.ReasonProgressing, "Installing components"), + subCondition("RevisionControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("RevisionControllerProgressing", configv1.ConditionTrue, operatorstatus.ReasonProgressing, "Updating revisions"), + }, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonAsExpected), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonProgressing). + WithMessage(SatisfyAll( + ContainSubstring("InstallerController"), + ContainSubstring("RevisionController"), + )), + defaultNodeTimeout), + Entry("when installer controller has a non-retryable error", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + subCondition("InstallerControllerAvailable", configv1.ConditionFalse, operatorstatus.ReasonNonRetryableError, "install failed"), + subCondition("InstallerControllerProgressing", configv1.ConditionFalse, operatorstatus.ReasonNonRetryableError, "install failed"), + subCondition("RevisionControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("RevisionControllerProgressing", configv1.ConditionFalse, operatorstatus.ReasonAsExpected, "Success"), + }, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionFalse). + WithReason(operatorstatus.ReasonNonRetryableError). + WithMessage(ContainSubstring("InstallerController")), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionFalse). + WithReason(operatorstatus.ReasonNonRetryableError). + WithMessage(ContainSubstring("InstallerController")), + defaultNodeTimeout), + Entry("when revision controller has a non-retryable error", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + subCondition("InstallerControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("InstallerControllerProgressing", configv1.ConditionFalse, operatorstatus.ReasonAsExpected, "Success"), + subCondition("RevisionControllerAvailable", configv1.ConditionFalse, operatorstatus.ReasonNonRetryableError, "revision failed"), + subCondition("RevisionControllerProgressing", configv1.ConditionFalse, operatorstatus.ReasonNonRetryableError, "revision failed"), + }, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionFalse). + WithReason(operatorstatus.ReasonNonRetryableError). + WithMessage(ContainSubstring("RevisionController")), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionFalse). + WithReason(operatorstatus.ReasonNonRetryableError). + WithMessage(ContainSubstring("RevisionController")), + defaultNodeTimeout), + Entry("when installer controller has an ephemeral error but was previously available", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + subCondition("InstallerControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("InstallerControllerProgressing", configv1.ConditionTrue, operatorstatus.ReasonEphemeralError, "transient failure"), + subCondition("RevisionControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("RevisionControllerProgressing", configv1.ConditionFalse, operatorstatus.ReasonAsExpected, "Success"), + }, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonAsExpected), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonEphemeralError). + WithMessage(ContainSubstring("InstallerController")), + defaultNodeTimeout), + Entry("when installer controller sub-conditions are missing", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + subCondition("RevisionControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("RevisionControllerProgressing", configv1.ConditionFalse, operatorstatus.ReasonAsExpected, "Success"), + }, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionFalse). + WithReason(operatorstatus.ReasonUninitialized). + WithMessage(ContainSubstring("InstallerController")), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonUninitialized). + WithMessage(ContainSubstring("InstallerController")), + defaultNodeTimeout), + Entry("when all sub-conditions are missing", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{}, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionFalse). + WithReason(operatorstatus.ReasonUninitialized). + WithMessage(SatisfyAll( + ContainSubstring("InstallerController"), + ContainSubstring("RevisionController"), + ContainSubstring("initializing"), + )), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonUninitialized). + WithMessage(SatisfyAll( + ContainSubstring("InstallerController"), + ContainSubstring("RevisionController"), + ContainSubstring("initializing"), + )), + defaultNodeTimeout), + Entry("when installer has not yet reported available during initial install", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + subCondition("InstallerControllerProgressing", configv1.ConditionTrue, operatorstatus.ReasonProgressing, "Installing components"), + }, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionFalse). + WithReason(operatorstatus.ReasonUninitialized). + WithMessage(SatisfyAll( + ContainSubstring("InstallerController"), + ContainSubstring("RevisionController"), + )), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonProgressing). + WithMessage(SatisfyAll( + ContainSubstring("InstallerController"), + ContainSubstring("RevisionController"), + )), + defaultNodeTimeout), + Entry("when one controller has a non-retryable error and the other is progressing", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + subCondition("InstallerControllerAvailable", configv1.ConditionFalse, operatorstatus.ReasonNonRetryableError, "install failed"), + subCondition("InstallerControllerProgressing", configv1.ConditionFalse, operatorstatus.ReasonNonRetryableError, "install failed"), + subCondition("RevisionControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("RevisionControllerProgressing", configv1.ConditionTrue, operatorstatus.ReasonProgressing, "Updating revisions"), + }, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionFalse). + WithReason(operatorstatus.ReasonNonRetryableError). + WithMessage(ContainSubstring("InstallerController")), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonNonRetryableError). + WithMessage(SatisfyAll( + ContainSubstring("InstallerController"), + ContainSubstring("RevisionController"), + )), + defaultNodeTimeout), + Entry("when both controllers have non-retryable errors", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + subCondition("InstallerControllerAvailable", configv1.ConditionFalse, operatorstatus.ReasonNonRetryableError, "install failed"), + subCondition("InstallerControllerProgressing", configv1.ConditionFalse, operatorstatus.ReasonNonRetryableError, "install failed"), + subCondition("RevisionControllerAvailable", configv1.ConditionFalse, operatorstatus.ReasonNonRetryableError, "revision failed"), + subCondition("RevisionControllerProgressing", configv1.ConditionFalse, operatorstatus.ReasonNonRetryableError, "revision failed"), + }, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionFalse). + WithReason(operatorstatus.ReasonNonRetryableError). + WithMessage(SatisfyAll( + ContainSubstring("InstallerController"), + ContainSubstring("RevisionController"), + )), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionFalse). + WithReason(operatorstatus.ReasonNonRetryableError). + WithMessage(SatisfyAll( + ContainSubstring("InstallerController"), + ContainSubstring("RevisionController"), + )), + defaultNodeTimeout), + Entry("when installer is waiting on external", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + subCondition("InstallerControllerProgressing", configv1.ConditionTrue, operatorstatus.ReasonWaitingOnExternal, "Waiting on ClusterAPI"), + }, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionFalse). + WithReason(operatorstatus.ReasonUninitialized). + WithMessage(SatisfyAll( + ContainSubstring("InstallerController"), + ContainSubstring("RevisionController"), + )), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonWaitingOnExternal). + WithMessage(SatisfyAll( + ContainSubstring("InstallerController"), + ContainSubstring("RevisionController"), + )), + defaultNodeTimeout), + + // Prioritisation tests: when sub-controllers report different reasons, + // the aggregated condition should report the highest priority reason. + Entry("should report the highest priority progressing reason", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + subCondition("InstallerControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("InstallerControllerProgressing", configv1.ConditionTrue, operatorstatus.ReasonEphemeralError, "transient failure"), + subCondition("RevisionControllerAvailable", configv1.ConditionTrue, operatorstatus.ReasonAsExpected, "Success"), + subCondition("RevisionControllerProgressing", configv1.ConditionTrue, operatorstatus.ReasonProgressing, "Updating revisions"), + }, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonAsExpected), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonEphemeralError). + WithMessage(SatisfyAll( + ContainSubstring("InstallerController"), + ContainSubstring("RevisionController"), + )), + defaultNodeTimeout), + Entry("should report the highest priority available reason", + []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ + subCondition("InstallerControllerAvailable", configv1.ConditionFalse, operatorstatus.ReasonNonRetryableError, "install failed"), + subCondition("InstallerControllerProgressing", configv1.ConditionFalse, operatorstatus.ReasonNonRetryableError, "install failed"), + subCondition("RevisionControllerAvailable", configv1.ConditionFalse, operatorstatus.ReasonUninitialized, ""), + subCondition("RevisionControllerProgressing", configv1.ConditionTrue, operatorstatus.ReasonProgressing, "Updating revisions"), + }, + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionFalse). + WithReason(operatorstatus.ReasonNonRetryableError). + WithMessage(SatisfyAll( + ContainSubstring("InstallerController"), + ContainSubstring("RevisionController"), + )), + test.HaveCondition(configv1.OperatorProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonNonRetryableError). + WithMessage(SatisfyAll( + ContainSubstring("InstallerController"), + ContainSubstring("RevisionController"), + )), + defaultNodeTimeout), + ) }) - AfterEach(func() { - testutils.CleanupResources(Default, ctx, testEnv.Config, cl, testNamespaceName, &configv1.ClusterOperator{}) - }) - - Context("With a supported platform", func() { - JustBeforeEach(func() { - mgrCancel, mgrDone = startManager(false) - }) - - JustAfterEach(func() { - stopManager() - }) - - It("should update the ClusterOperator status with the running version", func() { - co := komega.Object(configv1resourcebuilder.ClusterOperator().WithName(controllers.ClusterOperatorName).Build()) - Eventually(co).Should(HaveField("Status.Conditions", - SatisfyAll( - ContainElement(And(HaveField("Type", Equal(configv1.OperatorAvailable)), HaveField("Status", Equal(configv1.ConditionTrue)), - HaveField("Message", Equal(fmt.Sprintf("Cluster CAPI Operator is available at %s", desiredOperatorReleaseVersion))))), - ContainElement(And(HaveField("Type", Equal(configv1.OperatorProgressing)), HaveField("Status", Equal(configv1.ConditionFalse)))), - ContainElement(And(HaveField("Type", Equal(configv1.OperatorDegraded)), HaveField("Status", Equal(configv1.ConditionFalse)))), - ContainElement(And(HaveField("Type", Equal(configv1.OperatorUpgradeable)), HaveField("Status", Equal(configv1.ConditionTrue)))), - ), - ), "should match the expected ClusterOperator status conditions") - }) - - It("should update the ClusterOperator status version to the desired one", func() { - Eventually(komega.Object(configv1resourcebuilder.ClusterOperator().WithName(controllers.ClusterOperatorName).Build()), time.Second*10).Should( - HaveField("Status.Versions", ContainElement(SatisfyAll( - HaveField("Name", Equal("operator")), - HaveField("Version", Equal(desiredOperatorReleaseVersion)), - ))), - ) - }) - - It("should update the ClusterOperator status version to the desired one when an incorrect one is present", func() { - By("setting the ClusterOperator status version to an incorrect one") - - patchBase := client.MergeFrom(capiClusterOperator.DeepCopy()) - capiClusterOperator.Status.Versions = []configv1.OperandVersion{{Name: "operator", Version: "incorrect"}} - Expect(cl.Status().Patch(ctx, capiClusterOperator, patchBase)).To(Succeed()) - - co := komega.Object(configv1resourcebuilder.ClusterOperator().WithName(controllers.ClusterOperatorName).Build()) - Eventually(co).Should( - HaveField("Status.Versions", ContainElement(SatisfyAll( - HaveField("Name", Equal("operator")), - HaveField("Version", Equal(desiredOperatorReleaseVersion)), - ))), - ) - }) - }) + Context("with an unsupported platform", Ordered, func() { + var capiClusterOperator *configv1.ClusterOperator - Context("With an unsupported platform", func() { - JustBeforeEach(func() { + BeforeAll(func() { mgrCancel, mgrDone = startManager(true) - }) - - JustAfterEach(func() { - stopManager() - }) - It("should update the ClusterOperator status with an 'unsupported' message", func() { - Eventually(komega.Object(configv1resourcebuilder.ClusterOperator().WithName(controllers.ClusterOperatorName).Build())). - Should(HaveField("Status.Conditions", SatisfyAll( - ContainElement(And(HaveField("Type", Equal(configv1.OperatorAvailable)), HaveField("Status", Equal(configv1.ConditionTrue)), - HaveField("Message", Equal("Cluster API is not yet implemented on this platform")))), - ContainElement(And(HaveField("Type", Equal(configv1.OperatorProgressing)), HaveField("Status", Equal(configv1.ConditionFalse)))), - ContainElement(And(HaveField("Type", Equal(configv1.OperatorDegraded)), HaveField("Status", Equal(configv1.ConditionFalse)))), - ContainElement(And(HaveField("Type", Equal(configv1.OperatorUpgradeable)), HaveField("Status", Equal(configv1.ConditionTrue)))), - )), "should match the expected ClusterOperator status conditions") + DeferCleanup(stopManager) }) - It("should update the ClusterOperator status version to the desired one", func() { - Eventually(komega.Object(configv1resourcebuilder.ClusterOperator().WithName(controllers.ClusterOperatorName).Build())). - Should(HaveField("Status.Versions", ContainElement(SatisfyAll( - HaveField("Name", Equal("operator")), - HaveField("Version", Equal(desiredOperatorReleaseVersion)), - ))), "should match the expected ClusterOperator status versions") - }) - - It("should update the ClusterOperator status version to the desired one when an incorrect one is present", func() { + BeforeEach(func(ctx context.Context) { + By("Creating the cluster-api ClusterOperator", func() { + capiClusterOperator = &configv1.ClusterOperator{ + ObjectMeta: metav1.ObjectMeta{ + Name: controllers.ClusterOperatorName, + }, + } + Expect(cl.Create(ctx, capiClusterOperator)).To(Succeed()) + DeferCleanup(func(ctx context.Context) { + testutils.CleanupResources(Default, ctx, testEnv.Config, cl, "", &configv1.ClusterOperator{}) + }) + }) + }, defaultNodeTimeout) + + It("should set Available=True with unsupported message and write versions without reading sub-conditions", func(ctx context.Context) { + co := kWithCtx(ctx).Object(configv1resourcebuilder.ClusterOperator().WithName(controllers.ClusterOperatorName).Build()) + + Eventually(co). + WithContext(ctx). + WithTimeout(defaultEventuallyTimeout). + Should(SatisfyAll( + HaveField("Status.Conditions", SatisfyAll( + test.HaveCondition(configv1.OperatorAvailable). + WithStatus(configv1.ConditionTrue). + WithMessage(capiUnsupportedPlatformMsg), + test.HaveCondition(configv1.OperatorProgressing).WithStatus(configv1.ConditionFalse), + test.HaveCondition(configv1.OperatorDegraded).WithStatus(configv1.ConditionFalse), + test.HaveCondition(configv1.OperatorUpgradeable).WithStatus(configv1.ConditionTrue), + )), + HaveField("Status.Versions", ContainElement(SatisfyAll( + HaveField("Name", Equal(operatorstatus.OperatorVersionKey)), + HaveField("Version", Equal(desiredOperatorReleaseVersion)), + ))), + )) + }, defaultNodeTimeout) + + It("should update an incorrect version", func(ctx context.Context) { By("Setting the ClusterOperator status version to an incorrect one") patchBase := client.MergeFrom(capiClusterOperator.DeepCopy()) - capiClusterOperator.Status.Versions = []configv1.OperandVersion{{Name: "operator", Version: "incorrect"}} + capiClusterOperator.Status.Versions = []configv1.OperandVersion{{Name: operatorstatus.OperatorVersionKey, Version: "old"}} Expect(cl.Status().Patch(ctx, capiClusterOperator, patchBase)).To(Succeed()) - By("Checking the conditions are as expected") - Eventually(komega.Object(configv1resourcebuilder.ClusterOperator().WithName(controllers.ClusterOperatorName).Build())). + By("Checking the version is corrected") + Eventually(kWithCtx(ctx).Object(configv1resourcebuilder.ClusterOperator().WithName(controllers.ClusterOperatorName).Build())). + WithContext(ctx). + WithTimeout(defaultEventuallyTimeout). Should(HaveField("Status.Versions", ContainElement(SatisfyAll( - HaveField("Name", Equal("operator")), + HaveField("Name", Equal(operatorstatus.OperatorVersionKey)), HaveField("Version", Equal(desiredOperatorReleaseVersion)), )))) - }) + }, defaultNodeTimeout) }) }) +func patchSubConditions(ctx context.Context, co *configv1.ClusterOperator, conditions ...*configv1apply.ClusterOperatorStatusConditionApplyConfiguration) { + applyConfig := configv1apply.ClusterOperator(controllers.ClusterOperatorName). + WithUID(co.UID). + WithStatus(configv1apply.ClusterOperatorStatus(). + WithConditions(conditions...)) + + Expect(cl.Status().Patch(ctx, co, util.ApplyConfigPatch(applyConfig), + operatorstatus.CAPIFieldOwner("test-sub-conditions"), client.ForceOwnership)).To(Succeed()) +} + +func subCondition(condType string, status configv1.ConditionStatus, reason operatorstatus.Reason, message string) *configv1apply.ClusterOperatorStatusConditionApplyConfiguration { + return configv1apply.ClusterOperatorStatusCondition(). + WithType(configv1.ClusterStatusConditionType(condType)). + WithStatus(status). + WithReason(reason.String()). + WithMessage(message). + WithLastTransitionTime(metav1.Now()) +} + func startManager(isUnsupportedPlatform bool) (context.CancelFunc, chan struct{}) { mgrCtx, mgrCancel := context.WithCancel(context.Background()) mgrDone := make(chan struct{}) @@ -184,8 +457,9 @@ func startManager(isUnsupportedPlatform bool) (context.CancelFunc, chan struct{} Expect(err).ToNot(HaveOccurred(), "Manager should be able to be created") r := &ClusterOperatorController{ - ClusterOperatorStatusClient: operatorstatus.ClusterOperatorStatusClient{Client: cl, ReleaseVersion: desiredOperatorReleaseVersion}, - IsUnsupportedPlatform: isUnsupportedPlatform, + Client: cl, + ReleaseVersion: desiredOperatorReleaseVersion, + IsUnsupportedPlatform: isUnsupportedPlatform, } Expect(r.SetupWithManager(mgr)).To(Succeed(), "Reconciler should be able to setup with manager") @@ -201,8 +475,9 @@ func startManager(isUnsupportedPlatform bool) (context.CancelFunc, chan struct{} return mgrCancel, mgrDone } -func stopManager() { - By("Stopping the manager") - mgrCancel() - Eventually(mgrDone).Should(BeClosed()) +func stopManager(ctx context.Context) { + By("Stopping the manager", func() { + mgrCancel() + Eventually(mgrDone).WithContext(ctx).WithTimeout(defaultEventuallyTimeout).Should(BeClosed()) + }) } diff --git a/pkg/controllers/clusteroperator/suite_test.go b/pkg/controllers/clusteroperator/suite_test.go index 84f7acae31..4e5e87174c 100644 --- a/pkg/controllers/clusteroperator/suite_test.go +++ b/pkg/controllers/clusteroperator/suite_test.go @@ -19,6 +19,7 @@ package clusteroperator import ( "context" "testing" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -33,14 +34,22 @@ import ( "github.com/openshift/cluster-capi-operator/pkg/test" ) +var ( + defaultNodeTimeout = NodeTimeout(15 * time.Second) + defaultEventuallyTimeout = 5 * time.Second +) + var ( testEnv *envtest.Environment cfg *rest.Config cl client.Client testScheme *runtime.Scheme - ctx = context.Background() ) +func kWithCtx(ctx context.Context) komega.Komega { + return komega.New(cl).WithContext(ctx) +} + func TestAPIs(t *testing.T) { RegisterFailHandler(Fail) RunSpecs(t, "Controller Suite") @@ -49,21 +58,21 @@ func TestAPIs(t *testing.T) { var _ = BeforeSuite(func() { logf.SetLogger(klog.Background()) - By("bootstrapping test environment") + By("bootstrapping test environment", func() { + var err error - var err error + testEnv = &envtest.Environment{} + cfg, cl, err = test.StartEnvTest(testEnv) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg).NotTo(BeNil()) + Expect(cl).NotTo(BeNil()) + }) - testEnv = &envtest.Environment{} - cfg, cl, err = test.StartEnvTest(testEnv) - Expect(err).NotTo(HaveOccurred()) - Expect(cfg).NotTo(BeNil()) - Expect(cl).NotTo(BeNil()) + DeferCleanup(func() { + By("tearing down the test environment", func() { + Expect(test.StopEnvTest(testEnv)).To(Succeed()) + }) + }) komega.SetClient(cl) - komega.SetContext(ctx) -}) - -var _ = AfterSuite(func() { - By("tearing down the test environment") - Expect(test.StopEnvTest(testEnv)).To(Succeed()) }) diff --git a/pkg/controllers/common_consts.go b/pkg/controllers/common_consts.go index 99ad82cc03..e5c2883091 100644 --- a/pkg/controllers/common_consts.go +++ b/pkg/controllers/common_consts.go @@ -31,9 +31,6 @@ const ( // DefaultOperatorNamespace is the default namespace used for operator resources. DefaultOperatorNamespace = "openshift-cluster-api-operator" - // OperatorVersionKey is the key used to store the operator version in the ClusterOperator status. - OperatorVersionKey = "operator" - // ClusterOperatorName is the name of the ClusterOperator resource. ClusterOperatorName = "cluster-api" diff --git a/pkg/controllers/installer/installer_controller.go b/pkg/controllers/installer/installer_controller.go index 832d903201..cbd3036f34 100644 --- a/pkg/controllers/installer/installer_controller.go +++ b/pkg/controllers/installer/installer_controller.go @@ -54,7 +54,8 @@ const ( controllerName = "InstallerController" clusterAPIName = "cluster" - opresult = operatorstatus.ControllerResultGenerator(controllerName) + // ResultGenerator is the controller result generator for the InstallerController. + ResultGenerator = operatorstatus.ControllerResultGenerator(controllerName) ) // InstallerController reconciles ClusterAPI revisions, using boxcutter to apply @@ -205,18 +206,18 @@ func (c *InstallerController) reconcile(ctx context.Context, log logr.Logger) op clusterAPI := &operatorv1alpha1.ClusterAPI{} if err := c.client.Get(ctx, client.ObjectKey{Name: clusterAPIName}, clusterAPI); err != nil { if apierrors.IsNotFound(err) { - return opresult.WaitingOnExternal("ClusterAPI") + return ResultGenerator.WaitingOnExternal("ClusterAPI") } - return opresult.Error(fmt.Errorf("fetching ClusterAPI: %w", err)) + return ResultGenerator.Error(fmt.Errorf("fetching ClusterAPI: %w", err)) } if len(clusterAPI.Status.Revisions) == 0 { if err := writeRelatedObjects(ctx, c.client, staticRelatedObjects()); err != nil { - return opresult.Error(fmt.Errorf("writing relatedObjects: %w", err)) + return ResultGenerator.Error(fmt.Errorf("writing relatedObjects: %w", err)) } - return opresult.WaitingOnExternal("ClusterAPI revisions") + return ResultGenerator.WaitingOnExternal("ClusterAPI revisions") } revisionReconciler := newRevisionReconciler(c, log) @@ -226,12 +227,12 @@ func (c *InstallerController) reconcile(ctx context.Context, log logr.Logger) op // write never claims ownership of the relatedObjects field. relatedObjects := mergeRelatedObjects(staticRelatedObjects(), revisionReconciler.dynamicRelatedObjects()) if err := writeRelatedObjects(ctx, c.client, relatedObjects); err != nil { - return opresult.Error(fmt.Errorf("writing relatedObjects: %w", err)) + return ResultGenerator.Error(fmt.Errorf("writing relatedObjects: %w", err)) } // Update tracking cache watches for all current revisions if err := c.updateWatches(ctx, log, clusterAPI, revisionReconciler.gvks); err != nil { - return opresult.Error(err) + return ResultGenerator.Error(err) } if reconciledRevision != nil { @@ -242,7 +243,7 @@ func (c *InstallerController) reconcile(ctx context.Context, log logr.Logger) op return c.error(errs) } - return opresult.Progressing(strings.Join(messages, "\n")) + return ResultGenerator.Progressing(strings.Join(messages, "\n")) } func (c *InstallerController) success(ctx context.Context, log logr.Logger, clusterAPI *operatorv1alpha1.ClusterAPI, reconciledRevision operatorv1alpha1.RevisionName, errs []error) operatorstatus.ReconcileResult { @@ -252,10 +253,10 @@ func (c *InstallerController) success(ctx context.Context, log logr.Logger, clus // Write the current revision to the ClusterAPI status in its own SSA transaction if err := c.writeCurrentRevision(ctx, clusterAPI, reconciledRevision); err != nil { - return opresult.Error(fmt.Errorf("writing current revision: %w", err)) + return ResultGenerator.Error(fmt.Errorf("writing current revision: %w", err)) } - return opresult.Success() + return ResultGenerator.Success() } func (c *InstallerController) error(errs []error) operatorstatus.ReconcileResult { @@ -286,10 +287,10 @@ func (c *InstallerController) error(errs []error) operatorstatus.ReconcileResult }) nonTerminalErrors = append(nonTerminalErrors, unwrappedTerminalErrors...) - return opresult.Error(fmt.Errorf("reconciling revisions: %w", errors.Join(nonTerminalErrors...))) + return ResultGenerator.Error(fmt.Errorf("reconciling revisions: %w", errors.Join(nonTerminalErrors...))) } - return opresult.NonRetryableError(fmt.Errorf("reconciling revisions: %w", errors.Join(errs...))) + return ResultGenerator.NonRetryableError(fmt.Errorf("reconciling revisions: %w", errors.Join(errs...))) } func (c *InstallerController) updateWatches(ctx context.Context, log logr.Logger, clusterAPI *operatorv1alpha1.ClusterAPI, allGVKs sets.Set[schema.GroupVersionKind]) error { diff --git a/pkg/controllers/installer/related_objects_test.go b/pkg/controllers/installer/related_objects_test.go index 0d92267ac6..27584a03ed 100644 --- a/pkg/controllers/installer/related_objects_test.go +++ b/pkg/controllers/installer/related_objects_test.go @@ -28,7 +28,7 @@ import ( func TestStaticRelatedObjectsMatchManifest(t *testing.T) { // This test ensures that staticRelatedObjects() and the ClusterOperator // manifest do not drift. - data, err := os.ReadFile("../../../manifests/0000_30_cluster-api_12_clusteroperator.yaml") + data, err := os.ReadFile("../../../manifests/0000_30_cluster-api-operator_08_clusteroperator.yaml") if err != nil { t.Fatalf("reading ClusterOperator manifest: %v", err) } diff --git a/pkg/controllers/installerdeployment/assets/deployment.yaml b/pkg/controllers/installerdeployment/assets/deployment.yaml new file mode 100644 index 0000000000..14726f27ec --- /dev/null +++ b/pkg/controllers/installerdeployment/assets/deployment.yaml @@ -0,0 +1,80 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: capi-installer + annotations: + config.openshift.io/inject-proxy: capi-installer + labels: + k8s-app: capi-installer +spec: + selector: + matchLabels: + k8s-app: capi-installer + replicas: 1 + template: + metadata: + annotations: + target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' + openshift.io/required-scc: restricted-v2 + labels: + k8s-app: capi-installer + spec: + serviceAccountName: capi-installer + containers: + - name: capi-installer + command: + - /capi-installer + args: + - --diagnostics-address=:8443 + env: + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + ports: + - containerPort: 9440 + name: health + protocol: TCP + - containerPort: 8443 + name: diagnostics + protocol: TCP + resources: + requests: + cpu: 10m + memory: 50Mi + terminationMessagePolicy: FallbackToLogsOnError + volumeMounts: + - name: metrics-cert + mountPath: /tmp/k8s-metrics-server/serving-certs + readOnly: true + livenessProbe: + httpGet: + path: /healthz + port: 9440 + initialDelaySeconds: 15 + periodSeconds: 20 + readinessProbe: + httpGet: + path: /readyz + port: 9440 + initialDelaySeconds: 5 + periodSeconds: 10 + nodeSelector: + node-role.kubernetes.io/control-plane: "" + restartPolicy: Always + tolerations: + - key: "node-role.kubernetes.io/master" + operator: "Exists" + effect: "NoSchedule" + - key: "node-role.kubernetes.io/control-plane" + operator: "Exists" + effect: "NoSchedule" + volumes: + - name: metrics-cert + secret: + defaultMode: 420 + secretName: capi-installer-metrics-tls diff --git a/pkg/controllers/installerdeployment/controller.go b/pkg/controllers/installerdeployment/controller.go new file mode 100644 index 0000000000..d2629a94ef --- /dev/null +++ b/pkg/controllers/installerdeployment/controller.go @@ -0,0 +1,223 @@ +/* +Copyright 2026 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package installerdeployment + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1" + "github.com/openshift/cluster-capi-operator/pkg/providerimages" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +const ( + fieldManager = "capi-operator-installer-deployment" + clusterAPIName = "cluster" +) + +// InstallerDeploymentReconciler reconciles the capi-installer Deployment. +type InstallerDeploymentReconciler struct { + client.Client + Namespace string + ContainerImage string + SupportedPlatform bool +} + +// Reconcile reconciles the capi-installer Deployment by reading provider image refs +// from the ConfigMap and ClusterAPI revisions, then applying the desired deployment. +// On unsupported platforms, it deletes the deployment if it exists. +func (r *InstallerDeploymentReconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { + log := ctrl.LoggerFrom(ctx).WithName("InstallerDeploymentReconciler") + + // If platform is unsupported, delete deployment. + if !r.SupportedPlatform { + return r.deleteDeploymentIfExists(ctx, log) + } + + // Read ConfigMap with current-release provider image refs. + configMap, err := r.getConfigMap(ctx, log) + if err != nil { + return reconcile.Result{}, fmt.Errorf("failed to get ConfigMap: %w", err) + } + + configMapRefs, err := providerimages.ImageRefsFromConfigMap(configMap) + if err != nil { + return reconcile.Result{}, fmt.Errorf("failed to extract image refs from ConfigMap: %w", err) + } + + // Read ClusterAPI to get old revision image refs. + clusterAPI, err := r.getClusterAPI(ctx, log) + if err != nil { + return reconcile.Result{}, fmt.Errorf("failed to get ClusterAPI: %w", err) + } + + revisionRefs := providerimages.ImageRefsFromRevisions(clusterAPI.Status.Revisions) + + // Union all distinct image refs. + allImageRefs := configMapRefs.Union(revisionRefs) + + // Build desired deployment. + desired := buildDesiredDeployment(r.ContainerImage, r.Namespace, allImageRefs) + + // Apply deployment using Server-Side Apply. + if err := r.applyDeployment(ctx, log, desired); err != nil { + return reconcile.Result{}, fmt.Errorf("failed to apply Deployment: %w", err) + } + + log.Info("Successfully reconciled capi-installer Deployment") + + return reconcile.Result{}, nil +} + +// getConfigMap retrieves the capi-installer-images ConfigMap. +func (r *InstallerDeploymentReconciler) getConfigMap(ctx context.Context, log logr.Logger) (*corev1.ConfigMap, error) { + configMap := &corev1.ConfigMap{} + key := types.NamespacedName{ + Name: providerimages.ConfigMapName, + Namespace: r.Namespace, + } + + if err := r.Get(ctx, key, configMap); err != nil { + if apierrors.IsNotFound(err) { + log.Info("ConfigMap not found, using empty image refs", "name", key.Name) + + return &corev1.ConfigMap{Data: map[string]string{}}, nil + } + + return nil, fmt.Errorf("failed to get ConfigMap: %w", err) + } + + return configMap, nil +} + +// getClusterAPI retrieves the ClusterAPI singleton. +func (r *InstallerDeploymentReconciler) getClusterAPI(ctx context.Context, log logr.Logger) (*operatorv1alpha1.ClusterAPI, error) { + clusterAPI := &operatorv1alpha1.ClusterAPI{} + key := types.NamespacedName{ + Name: clusterAPIName, + } + + if err := r.Get(ctx, key, clusterAPI); err != nil { + if apierrors.IsNotFound(err) { + log.Info("ClusterAPI not found, using empty revisions") + + return &operatorv1alpha1.ClusterAPI{}, nil + } + + return nil, fmt.Errorf("failed to get ClusterAPI: %w", err) + } + + return clusterAPI, nil +} + +// applyDeployment applies the Deployment using Server-Side Apply. +func (r *InstallerDeploymentReconciler) applyDeployment(ctx context.Context, log logr.Logger, desired *appsv1.Deployment) error { + // Ensure TypeMeta is set for SSA + desired.TypeMeta = metav1.TypeMeta{ + APIVersion: appsv1.SchemeGroupVersion.String(), + Kind: "Deployment", + } + + if err := r.Patch(ctx, desired, client.Apply, &client.PatchOptions{ + FieldManager: fieldManager, + Force: ptr.To(true), + }); err != nil { + return fmt.Errorf("failed to patch Deployment: %w", err) + } + + log.Info("Applied capi-installer Deployment", "name", desired.Name) + + return nil +} + +// SetupWithManager sets up the controller with the Manager. +func (r *InstallerDeploymentReconciler) SetupWithManager(mgr ctrl.Manager) error { + if err := ctrl.NewControllerManagedBy(mgr). + For(&appsv1.Deployment{}, builder.WithPredicates(predicate.NewPredicateFuncs(func(obj client.Object) bool { + return obj.GetName() == deploymentName + }))). + Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(r.mapConfigMapToReconcile)). + Watches(&operatorv1alpha1.ClusterAPI{}, handler.EnqueueRequestsFromMapFunc(r.mapClusterAPIToReconcile)). + Complete(r); err != nil { + return fmt.Errorf("failed to create controller: %w", err) + } + + return nil +} + +// mapConfigMapToReconcile maps ConfigMap events to reconcile requests. +func (r *InstallerDeploymentReconciler) mapConfigMapToReconcile(ctx context.Context, obj client.Object) []reconcile.Request { + if obj.GetName() == providerimages.ConfigMapName && obj.GetNamespace() == r.Namespace { + return []reconcile.Request{{NamespacedName: types.NamespacedName{ + Name: deploymentName, + Namespace: r.Namespace, + }}} + } + + return nil +} + +// mapClusterAPIToReconcile maps ClusterAPI events to reconcile requests. +func (r *InstallerDeploymentReconciler) mapClusterAPIToReconcile(ctx context.Context, obj client.Object) []reconcile.Request { + if obj.GetName() == clusterAPIName { + return []reconcile.Request{{NamespacedName: types.NamespacedName{ + Name: deploymentName, + Namespace: r.Namespace, + }}} + } + + return nil +} + +// deleteDeploymentIfExists deletes the capi-installer Deployment if it exists. +// Returns no error if the deployment doesn't exist. +func (r *InstallerDeploymentReconciler) deleteDeploymentIfExists(ctx context.Context, log logr.Logger) (reconcile.Result, error) { + deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: deploymentName, + Namespace: r.Namespace, + }, + } + + err := r.Delete(ctx, deployment) + if err != nil { + if apierrors.IsNotFound(err) { + log.V(1).Info("Deployment does not exist, nothing to delete") + + return reconcile.Result{}, nil + } + + return reconcile.Result{}, fmt.Errorf("failed to delete Deployment: %w", err) + } + + log.Info("Deleted capi-installer Deployment on unsupported platform") + + return reconcile.Result{}, nil +} diff --git a/pkg/controllers/installerdeployment/controller_test.go b/pkg/controllers/installerdeployment/controller_test.go new file mode 100644 index 0000000000..6a9d69aa53 --- /dev/null +++ b/pkg/controllers/installerdeployment/controller_test.go @@ -0,0 +1,269 @@ +/* +Copyright 2026 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package installerdeployment + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1" + "github.com/openshift/cluster-api-actuator-pkg/testutils" + "github.com/openshift/cluster-capi-operator/pkg/providerimages" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest/komega" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +const ( + testTimeout = 10 * time.Second + testInterval = 100 * time.Millisecond +) + +var _ = Describe("InstallerDeployment Controller", func() { + var ( + ctx context.Context + reconciler *InstallerDeploymentReconciler + configMap *corev1.ConfigMap + clusterAPI *operatorv1alpha1.ClusterAPI + k komega.Komega + namespace string + ) + + BeforeEach(func() { + ctx = context.Background() + k = komega.New(cl).WithContext(ctx) + + // Create a unique test namespace. + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-installer-", + }, + } + Expect(cl.Create(ctx, ns)).To(Succeed()) + + namespace = ns.Name + + // Create the InstallerDeploymentReconciler. + reconciler = &InstallerDeploymentReconciler{ + Client: cl, + Namespace: namespace, + ContainerImage: "quay.io/openshift/cluster-capi-operator:test", + SupportedPlatform: true, + } + + // Create ConfigMap with provider image refs. + configMap = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: providerimages.ConfigMapName, + Namespace: namespace, + }, + Data: map[string]string{ + "aws-cluster-api-controllers": "registry/aws@sha256:abc", + "core-cluster-api-controllers": "registry/core@sha256:def", + }, + } + Expect(cl.Create(ctx, configMap)).To(Succeed()) + + // Create ClusterAPI singleton. + clusterAPI = &operatorv1alpha1.ClusterAPI{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterAPIName, + }, + Spec: &operatorv1alpha1.ClusterAPISpec{}, + } + Expect(cl.Create(ctx, clusterAPI)).To(Succeed()) + + DeferCleanup(func() { + testutils.CleanupResources(Default, ctx, cfg, cl, namespace, + &corev1.ConfigMap{}, + &appsv1.Deployment{}, + ) + Expect(cl.Delete(ctx, clusterAPI)).To(Succeed()) + }) + }) + + It("should create a Deployment with image volumes for ConfigMap refs", func() { + _, err := reconciler.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + + // Verify deployment was created with correct number of image volumes + // (2 from ConfigMap + 1 metrics-cert from the embedded base). + Eventually(k.Object(&appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: deploymentName, + Namespace: namespace, + }, + })).WithTimeout(testTimeout).WithPolling(testInterval).Should(HaveField("Spec.Template.Spec.Volumes", HaveLen(3))) + + deployment := &appsv1.Deployment{} + Expect(cl.Get(ctx, client.ObjectKey{Name: deploymentName, Namespace: namespace}, deployment)).To(Succeed()) + + var imageRefs []string + + for _, vol := range deployment.Spec.Template.Spec.Volumes { + if vol.Image != nil { + imageRefs = append(imageRefs, vol.Image.Reference) + } + } + + Expect(imageRefs).To(ConsistOf("registry/aws@sha256:abc", "registry/core@sha256:def")) + }) + + It("should update Deployment when ConfigMap is updated", func() { + _, err := reconciler.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + + deployment := &appsv1.Deployment{} + + Eventually(func() error { + return cl.Get(ctx, client.ObjectKey{Name: deploymentName, Namespace: namespace}, deployment) + }).WithTimeout(testTimeout).WithPolling(testInterval).Should(Succeed()) + + Eventually(k.Update(configMap, func() { + configMap.Data["gcp-cluster-api-controllers"] = "registry/gcp@sha256:123" + })).Should(Succeed()) + + _, err = reconciler.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + + // Verify deployment has 4 volumes now (3 image + 1 metrics-cert). + Eventually(k.Object(&appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: deploymentName, + Namespace: namespace, + }, + })).WithTimeout(testTimeout).WithPolling(testInterval).Should(HaveField("Spec.Template.Spec.Volumes", HaveLen(4))) + + Expect(cl.Get(ctx, client.ObjectKey{Name: deploymentName, Namespace: namespace}, deployment)).To(Succeed()) + + var imageRefs []string + + for _, vol := range deployment.Spec.Template.Spec.Volumes { + if vol.Image != nil { + imageRefs = append(imageRefs, vol.Image.Reference) + } + } + + Expect(imageRefs).To(ContainElement("registry/gcp@sha256:123")) + }) + + It("should include old revision images not in ConfigMap", func() { + Eventually(k.UpdateStatus(clusterAPI, func() { + clusterAPI.Status.Revisions = []operatorv1alpha1.ClusterAPIInstallerRevision{ + { + Name: "rev-1", + Revision: 1, + ContentID: "old-content", + Components: []operatorv1alpha1.ClusterAPIInstallerComponent{ + { + Name: "old-provider", + ClusterAPIInstallerComponentSource: operatorv1alpha1.ClusterAPIInstallerComponentSource{ + Type: operatorv1alpha1.InstallerComponentTypeImage, + Image: operatorv1alpha1.ClusterAPIInstallerComponentImage{ + Ref: "registry.example.com/old@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + Profile: "default", + }, + }, + }, + }, + }, + } + clusterAPI.Status.DesiredRevision = "rev-1" + })).Should(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + + // Verify deployment has volumes for both ConfigMap and old revision images + // (2 ConfigMap + 1 revision + 1 metrics-cert = 4). + deployment := &appsv1.Deployment{} + + Eventually(func() int { + if err := cl.Get(ctx, client.ObjectKey{Name: deploymentName, Namespace: namespace}, deployment); err != nil { + return 0 + } + + return len(deployment.Spec.Template.Spec.Volumes) + }).WithTimeout(testTimeout).WithPolling(testInterval).Should(BeNumerically(">=", 4)) + + var imageRefs []string + + for _, vol := range deployment.Spec.Template.Spec.Volumes { + if vol.Image != nil { + imageRefs = append(imageRefs, vol.Image.Reference) + } + } + + Expect(imageRefs).To(ContainElement("registry.example.com/old@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef")) + }) + + Context("when platform is supported", func() { + It("should not error when reconciling with unchanged inputs", func() { + _, err := reconciler.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + + deployment := &appsv1.Deployment{} + + Eventually(func() error { + return cl.Get(ctx, client.ObjectKey{Name: deploymentName, Namespace: namespace}, deployment) + }).WithTimeout(testTimeout).WithPolling(testInterval).Should(Succeed()) + + _, err = reconciler.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + }) + }) + + Context("when platform is unsupported", func() { + It("should delete Deployment when it exists", func() { + _, err := reconciler.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + + deployment := &appsv1.Deployment{} + + Eventually(func() error { + return cl.Get(ctx, client.ObjectKey{Name: deploymentName, Namespace: namespace}, deployment) + }).WithTimeout(testTimeout).WithPolling(testInterval).Should(Succeed()) + + reconciler.SupportedPlatform = false + + _, err = reconciler.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + + Eventually(func() bool { + err := cl.Get(ctx, client.ObjectKey{Name: deploymentName, Namespace: namespace}, deployment) + + return err != nil + }).WithTimeout(testTimeout).WithPolling(testInterval).Should(BeTrue()) + }) + + It("should not error when Deployment does not exist", func() { + reconciler.SupportedPlatform = false + + _, err := reconciler.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + + deployment := &appsv1.Deployment{} + err = cl.Get(ctx, client.ObjectKey{Name: deploymentName, Namespace: namespace}, deployment) + Expect(err).To(HaveOccurred()) + }) + }) +}) diff --git a/pkg/controllers/installerdeployment/deployment.go b/pkg/controllers/installerdeployment/deployment.go new file mode 100644 index 0000000000..7781d9ef72 --- /dev/null +++ b/pkg/controllers/installerdeployment/deployment.go @@ -0,0 +1,103 @@ +/* +Copyright 2026 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package installerdeployment + +import ( + _ "embed" + "fmt" + "os" + + "github.com/openshift/cluster-capi-operator/pkg/providerimages" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/yaml" +) + +const ( + deploymentName = "capi-installer" +) + +var ( + //go:embed assets/deployment.yaml + deploymentYAML []byte + + // staticDeployment holds the parsed deployment YAML. + //nolint:gochecknoglobals + staticDeployment = appsv1.Deployment{} +) + +func init() { + // Parse the embedded deployment YAML on startup + if err := yaml.UnmarshalStrict(deploymentYAML, &staticDeployment); err != nil { + panic(fmt.Errorf("failed to parse embedded deployment YAML: %w", err)) + } +} + +// buildDesiredDeployment constructs the desired capi-installer Deployment spec +// by parsing the embedded YAML base and overlaying dynamic fields: container image, +// namespace, RELEASE_VERSION env var, and image volumes/mounts. +func buildDesiredDeployment(containerImage, namespace string, imageRefs sets.Set[string]) *appsv1.Deployment { + deployment := staticDeployment.DeepCopy() + + // Overlay dynamic fields + deployment.Namespace = namespace + deployment.Spec.Template.Spec.Containers[0].Image = containerImage + + // Add RELEASE_VERSION env var from the operator's own environment + releaseVersion := os.Getenv("RELEASE_VERSION") + if releaseVersion == "" { + releaseVersion = "0.0.1-snapshot" + } + + deployment.Spec.Template.Spec.Containers[0].Env = append( + deployment.Spec.Template.Spec.Containers[0].Env, + corev1.EnvVar{ + Name: "RELEASE_VERSION", + Value: releaseVersion, + }, + ) + + // Build image volumes and volume mounts from image refs. + // sets.List sorts image refs for deterministic output. + for _, imageRef := range sets.List(imageRefs) { + name := providerimages.VolumeNameForImageRef(imageRef) + + deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, + corev1.Volume{ + Name: name, + VolumeSource: corev1.VolumeSource{ + Image: &corev1.ImageVolumeSource{ + Reference: imageRef, + PullPolicy: corev1.PullIfNotPresent, + }, + }, + }, + ) + + deployment.Spec.Template.Spec.Containers[0].VolumeMounts = append( + deployment.Spec.Template.Spec.Containers[0].VolumeMounts, + corev1.VolumeMount{ + Name: name, + MountPath: fmt.Sprintf("%s/%s", providerimages.ProviderImageMountBase, name), + ReadOnly: true, + }, + ) + } + + return deployment +} diff --git a/pkg/controllers/installerdeployment/deployment_test.go b/pkg/controllers/installerdeployment/deployment_test.go new file mode 100644 index 0000000000..f99307c66f --- /dev/null +++ b/pkg/controllers/installerdeployment/deployment_test.go @@ -0,0 +1,110 @@ +/* +Copyright 2026 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package installerdeployment + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/util/sets" +) + +var _ = Describe("buildDesiredDeployment", func() { + const ( + testImage = "quay.io/openshift/cluster-capi-operator:latest" + ) + + It("should parse the embedded YAML and overlay dynamic fields", func() { + deployment := buildDesiredDeployment(testImage, testNamespace, sets.New[string]()) + + Expect(deployment.Name).To(Equal("capi-installer")) + Expect(deployment.Namespace).To(Equal(testNamespace)) + Expect(deployment.Spec.Template.Spec.Containers[0].Image).To(Equal(testImage)) + // Verify the static base content survived YAML parsing. + Expect(deployment.Spec.Template.Spec.ServiceAccountName).To(Equal("capi-installer")) + }) + + It("should create image volumes and mounts for all image refs", func() { + imageRefs := sets.New( + "registry/aws@sha256:abc", + "registry/core@sha256:def", + ) + + deployment := buildDesiredDeployment(testImage, testNamespace, imageRefs) + + volumes := deployment.Spec.Template.Spec.Volumes + // 2 image volumes + 1 metrics-cert volume from base. + Expect(volumes).To(HaveLen(3)) + + // Verify all image refs are present in volumes. + var volumeImageRefs []string + + for _, vol := range volumes { + if vol.Image != nil { + volumeImageRefs = append(volumeImageRefs, vol.Image.Reference) + } + } + + Expect(volumeImageRefs).To(ConsistOf( + "registry/aws@sha256:abc", + "registry/core@sha256:def", + )) + + // Verify volume mounts include both image mounts and metrics-cert. + container := deployment.Spec.Template.Spec.Containers[0] + // 2 image mounts + 1 metrics-cert mount from base. + Expect(container.VolumeMounts).To(HaveLen(3)) + }) + + It("should produce deterministic output when called multiple times", func() { + imageRefs := sets.New[string]( + "registry/gcp@sha256:123", + "registry/aws@sha256:abc", + "registry/core@sha256:def", + ) + + deployment1 := buildDesiredDeployment(testImage, testNamespace, imageRefs) + + deployment2 := buildDesiredDeployment(testImage, testNamespace, imageRefs) + + Expect(deployment1).To(Equal(deployment2)) + + // Verify image volumes are sorted by name for determinism. + var imageVolumeNames []string + + for _, vol := range deployment1.Spec.Template.Spec.Volumes { + if vol.Image != nil { + imageVolumeNames = append(imageVolumeNames, vol.Name) + } + } + + for i := 1; i < len(imageVolumeNames); i++ { + Expect(imageVolumeNames[i] > imageVolumeNames[i-1]).To(BeTrue()) + } + }) + + It("should have only base volumes when imageRefs is empty", func() { + deployment := buildDesiredDeployment(testImage, testNamespace, sets.New[string]()) + + // Only the metrics-cert volume from the base. + Expect(deployment.Spec.Template.Spec.Volumes).To(HaveLen(1)) + Expect(deployment.Spec.Template.Spec.Volumes[0].Name).To(Equal("metrics-cert")) + + // Only the metrics-cert volume mount from the base. + Expect(deployment.Spec.Template.Spec.Containers[0].VolumeMounts).To(HaveLen(1)) + Expect(deployment.Spec.Template.Spec.Containers[0].VolumeMounts[0].Name).To(Equal("metrics-cert")) + }) +}) diff --git a/pkg/controllers/installerdeployment/suite_test.go b/pkg/controllers/installerdeployment/suite_test.go new file mode 100644 index 0000000000..baebed8c8f --- /dev/null +++ b/pkg/controllers/installerdeployment/suite_test.go @@ -0,0 +1,67 @@ +/* +Copyright 2026 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package installerdeployment + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/openshift/cluster-capi-operator/pkg/test" + "k8s.io/client-go/rest" + "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + "sigs.k8s.io/controller-runtime/pkg/envtest/komega" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +var ( + testEnv *envtest.Environment + cfg *rest.Config + cl client.WithWatch +) + +const ( + testNamespace = "test-namespace" +) + +func TestInstallerDeploymentController(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "InstallerDeployment Controller Suite") +} + +var _ = BeforeSuite(func() { + logf.SetLogger(klog.Background()) + + By("bootstrapping test environment") + + var err error + + testEnv = &envtest.Environment{} + cfg, cl, err = test.StartEnvTest(testEnv) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg).NotTo(BeNil()) + Expect(cl).NotTo(BeNil()) + + komega.SetClient(cl) + + DeferCleanup(func() { + By("tearing down the test environment") + Expect(test.StopEnvTest(testEnv)).To(Succeed()) + }) +}) diff --git a/pkg/controllers/revision/revision_controller.go b/pkg/controllers/revision/revision_controller.go index d276ff8b97..a76d65a6e2 100644 --- a/pkg/controllers/revision/revision_controller.go +++ b/pkg/controllers/revision/revision_controller.go @@ -52,7 +52,8 @@ const ( infrastructureName = "cluster" maxRevisionsAllowed = 16 - opresult = operatorstatus.ControllerResultGenerator(controllerName) + // ResultGenerator is the controller result generator for the RevisionController. + ResultGenerator = operatorstatus.ControllerResultGenerator(controllerName) ) var ( @@ -99,10 +100,10 @@ func (r *RevisionController) reconcile(ctx context.Context, log logr.Logger) ope clusterAPI := &operatorv1alpha1.ClusterAPI{} if err := r.Get(ctx, client.ObjectKey{Name: clusterAPIName}, clusterAPI); err != nil { if apierrors.IsNotFound(err) { - return opresult.WaitingOnExternal("ClusterAPI not found") + return ResultGenerator.WaitingOnExternal("ClusterAPI not found") } - return opresult.Error(fmt.Errorf("fetching ClusterAPI: %w", err)) + return ResultGenerator.Error(fmt.Errorf("fetching ClusterAPI: %w", err)) } // Create a reverse sorted, merged list of revisions. It will prepend the @@ -110,7 +111,7 @@ func (r *RevisionController) reconcile(ctx context.Context, log logr.Logger) ope // first, and there is guaranteed to be at least one revision. apiRevisions, err := r.mergeRevisions(log, clusterAPI.Status.Revisions, desiredRevision) if err != nil { - return opresult.Error(fmt.Errorf("error merging revisions: %w", err)) + return ResultGenerator.Error(fmt.Errorf("error merging revisions: %w", err)) } // We can't proceed if we exceed the max number of revisions. In normal @@ -119,29 +120,36 @@ func (r *RevisionController) reconcile(ctx context.Context, log logr.Logger) ope // so we should stop. There is no safe way to automatically prune revisions // in this case. This requires manual intervention. if len(apiRevisions) > maxRevisionsAllowed { - return opresult.NonRetryableError(errMaxRevisionsAllowed) + return ResultGenerator.NonRetryableError(errMaxRevisionsAllowed) } + upToDate := clusterAPI.Status.CurrentRevision == apiRevisions[0].Name + // Trim old revisions if the current revision is up to date - if len(apiRevisions) > 0 && clusterAPI.Status.CurrentRevision == apiRevisions[0].Name { + if len(apiRevisions) > 0 && upToDate { apiRevisions = apiRevisions[:1] } if err := r.writeRevisions(ctx, log, clusterAPI, apiRevisions); err != nil { - return opresult.Error(fmt.Errorf("writing new revision: %w", err)) + return ResultGenerator.Error(fmt.Errorf("writing new revision: %w", err)) + } + + reconcileResult := ResultGenerator.Success() + if upToDate { + reconcileResult = reconcileResult.WithUpdateOperatorVersion(r.ReleaseVersion) } - return opresult.Success() + return reconcileResult } func (r *RevisionController) generateDesiredRevision(ctx context.Context) (revisiongenerator.RenderedRevision, *operatorstatus.ReconcileResult) { infra := &configv1.Infrastructure{} if err := r.Get(ctx, client.ObjectKey{Name: infrastructureName}, infra); err != nil { - return nil, opresult.ErrorP(fmt.Errorf("fetching infrastructure: %w", err)) + return nil, ResultGenerator.ErrorP(fmt.Errorf("fetching infrastructure: %w", err)) } if infra.Status.PlatformStatus == nil { - return nil, opresult.WaitingOnExternalP("Infrastructure PlatformStatus") + return nil, ResultGenerator.WaitingOnExternalP("Infrastructure PlatformStatus") } // Build ordered component list from provider metadata @@ -149,7 +157,7 @@ func (r *RevisionController) generateDesiredRevision(ctx context.Context) (revis revision, err := revisiongenerator.NewRenderedRevision(providerComponents, revisiongenerator.WithManifestSubstitutions(r.manifestSubstitutions)) if err != nil { - return nil, opresult.ErrorP(fmt.Errorf("error creating rendered revision: %w", err)) + return nil, ResultGenerator.ErrorP(fmt.Errorf("error creating rendered revision: %w", err)) } return revision, nil diff --git a/pkg/controllers/revision/revision_controller_test.go b/pkg/controllers/revision/revision_controller_test.go index c303e0e19e..06ee92b0f9 100644 --- a/pkg/controllers/revision/revision_controller_test.go +++ b/pkg/controllers/revision/revision_controller_test.go @@ -169,6 +169,9 @@ var _ = Describe("RevisionController", Serial, func() { Expect(co.Status.Conditions).To(test.HaveCondition(conditionTypeAvailable). WithStatus(configv1.ConditionTrue). WithReason(operatorstatus.ReasonAsExpected)) + + // Should NOT set operator version (not up to date — CurrentRevision is empty) + Expect(co.Status.Versions).To(BeEmpty()) }, defaultNodeTimeout) It("does not modify up to date revision list", func(ctx context.Context) { @@ -250,6 +253,11 @@ var _ = Describe("RevisionController", Serial, func() { // ObservedRevisionGeneration should match the ClusterAPI generation Expect(clusterAPI.Status.ObservedRevisionGeneration).To(Equal(clusterAPI.Generation)) + + // Should NOT set operator version (new revision not yet installed) + co := &configv1.ClusterOperator{} + Expect(cl.Get(ctx, client.ObjectKey{Name: "cluster-api"}, co)).To(Succeed()) + Expect(co.Status.Versions).To(BeEmpty()) }, defaultNodeTimeout) It("creates revision with empty components when no providers match the platform", func(ctx context.Context) { @@ -301,6 +309,14 @@ var _ = Describe("RevisionController", Serial, func() { Expect(clusterAPI.Status.Revisions[0].Name).To(Equal(latest.Name)) Expect(clusterAPI.Status.DesiredRevision).To(Equal(latest.Name)) Expect(clusterAPI.Status.ObservedRevisionGeneration).To(Equal(clusterAPI.Generation)) + + // Should set operator version (up to date) + co := &configv1.ClusterOperator{} + Expect(cl.Get(ctx, client.ObjectKey{Name: "cluster-api"}, co)).To(Succeed()) + Expect(co.Status.Versions).To(ContainElement(SatisfyAll( + HaveField("Name", Equal(operatorstatus.OperatorVersionKey)), + HaveField("Version", Equal("4.18.0")), + ))) }, defaultNodeTimeout) It("preserves all revisions when content changes and current is set", func(ctx context.Context) { @@ -327,6 +343,67 @@ var _ = Describe("RevisionController", Serial, func() { Expect(clusterAPI.Status.DesiredRevision).NotTo(Equal(rev1Name)) Expect(clusterAPI.Status.CurrentRevision).To(Equal(rev1Name)) Expect(clusterAPI.Status.ObservedRevisionGeneration).To(Equal(clusterAPI.Generation)) + + // Should NOT set operator version (current != latest) + co := &configv1.ClusterOperator{} + Expect(cl.Get(ctx, client.ObjectKey{Name: "cluster-api"}, co)).To(Succeed()) + Expect(co.Status.Versions).To(BeEmpty()) + }, defaultNodeTimeout) + + It("sets operator version when current revision matches latest", func(ctx context.Context) { + // BeforeEach created rev1. Set CurrentRevision to match. + Expect(kWithCtx(ctx).Get(clusterAPI)()).To(Succeed()) + Expect(clusterAPI.Status.Revisions).To(HaveLen(1)) + patch := client.MergeFrom(clusterAPI.DeepCopy()) + clusterAPI.Status.CurrentRevision = clusterAPI.Status.Revisions[0].Name + Expect(cl.Status().Patch(ctx, clusterAPI, patch)).To(Succeed()) + + // Trigger a reconcile + Eventually(kWithCtx(ctx).Update(clusterAPI, func() { + metav1.SetMetaDataAnnotation(&clusterAPI.ObjectMeta, "test", "trigger-version") + })).WithContext(ctx).Should(Succeed()) + + // Wait for the version to appear + co := &configv1.ClusterOperator{} + co.SetName("cluster-api") + Eventually(kWithCtx(ctx).Object(co)). + WithContext(ctx). + Should(HaveField("Status.Versions", ContainElement(SatisfyAll( + HaveField("Name", Equal(operatorstatus.OperatorVersionKey)), + HaveField("Version", Equal("4.18.0")), + )))) + }, defaultNodeTimeout) + + It("corrects stale operator version when current revision matches latest", func(ctx context.Context) { + // BeforeEach created rev1. Set CurrentRevision to match. + Expect(kWithCtx(ctx).Get(clusterAPI)()).To(Succeed()) + Expect(clusterAPI.Status.Revisions).To(HaveLen(1)) + patch := client.MergeFrom(clusterAPI.DeepCopy()) + clusterAPI.Status.CurrentRevision = clusterAPI.Status.Revisions[0].Name + Expect(cl.Status().Patch(ctx, clusterAPI, patch)).To(Succeed()) + + // Seed an incorrect operator version + coKey := client.ObjectKey{Name: "cluster-api"} + co := &configv1.ClusterOperator{} + Expect(cl.Get(ctx, coKey, co)).To(Succeed()) + coPatch := client.MergeFrom(co.DeepCopy()) + co.Status.Versions = []configv1.OperandVersion{{Name: operatorstatus.OperatorVersionKey, Version: "incorrect"}} + Expect(cl.Status().Patch(ctx, co, coPatch)).To(Succeed()) + + // Trigger a reconcile + Eventually(kWithCtx(ctx).Update(clusterAPI, func() { + metav1.SetMetaDataAnnotation(&clusterAPI.ObjectMeta, "test", "trigger-version-correction") + })).WithContext(ctx).Should(Succeed()) + + // Wait for the version to be corrected + co = &configv1.ClusterOperator{} + co.SetName("cluster-api") + Eventually(kWithCtx(ctx).Object(co)). + WithContext(ctx). + Should(HaveField("Status.Versions", ContainElement(SatisfyAll( + HaveField("Name", Equal(operatorstatus.OperatorVersionKey)), + HaveField("Version", Equal("4.18.0")), + )))) }, defaultNodeTimeout) It("sets Available=False with NonRetryableError when max revisions reached", func(ctx context.Context) { diff --git a/pkg/controllers/secretsync/secret_sync_controller.go b/pkg/controllers/secretsync/secret_sync_controller.go index e3c35749da..ff0dc8111a 100644 --- a/pkg/controllers/secretsync/secret_sync_controller.go +++ b/pkg/controllers/secretsync/secret_sync_controller.go @@ -214,9 +214,9 @@ func (r *UserDataSecretController) setDegradedCondition(ctx context.Context, log } conds := []configv1.ClusterOperatorStatusCondition{ - operatorstatus.NewClusterOperatorStatusCondition(secretSyncControllerAvailableCondition, configv1.ConditionFalse, operatorstatus.ReasonSyncFailed, + operatorstatus.NewClusterOperatorStatusCondition(secretSyncControllerAvailableCondition, configv1.ConditionFalse, operatorstatus.ReasonEphemeralError, "User Data Secret Controller failed to sync secret"), - operatorstatus.NewClusterOperatorStatusCondition(secretSyncControllerDegradedCondition, configv1.ConditionTrue, operatorstatus.ReasonSyncFailed, + operatorstatus.NewClusterOperatorStatusCondition(secretSyncControllerDegradedCondition, configv1.ConditionTrue, operatorstatus.ReasonEphemeralError, "User Data Secret Controller failed to sync secret"), } diff --git a/pkg/operatorstatus/controller_status.go b/pkg/operatorstatus/controller_status.go index 42928fd3c3..7316ac7e2d 100644 --- a/pkg/operatorstatus/controller_status.go +++ b/pkg/operatorstatus/controller_status.go @@ -41,34 +41,79 @@ const ( // server-side apply operations by the CAPI operator. CAPIOperatorIdentifierDomain = "capi-operator.openshift.io" + // ConditionAvailableSuffix is the suffix added to a controller prefix to + // form the controller's available condition type. + ConditionAvailableSuffix = "Available" + + // ConditionProgressingSuffix is the suffix added to a controller prefix to + // form the controller's progressing condition type. + ConditionProgressingSuffix = "Progressing" +) + +const ( + // OperatorVersionKey is the key used to store the operator version in the ClusterOperator status. + OperatorVersionKey = "operator" +) + +//go:generate go run golang.org/x/tools/cmd/stringer -type=Reason -trimprefix=Reason + +// Reason is a type that represents the reason for a condition. +type Reason int + +// Reasons are ordered by severity from least to most severe. When aggregating +// reasons, only the most severe reason will be reported. +const ( + // ReasonUnknown is the default reason for a condition when the reason is not known. + // Nothing should use this. + ReasonUnknown Reason = iota + // ReasonAsExpected is the reason for the condition when the operator is in a normal state. - ReasonAsExpected = "AsExpected" + ReasonAsExpected + + // ReasonUninitialized is the reason for the condition when the controller has not yet been initialized. + // This is used to indicate that the controller is not yet available. + ReasonUninitialized // ReasonProgressing indicates that the controller is progressing normally. // An observer should continue to wait. - ReasonProgressing = "Progressing" + ReasonProgressing // ReasonWaitingOnExternal indicates that the controller is waiting on an external event. // An observer should continue to wait. - ReasonWaitingOnExternal = "WaitingOnExternal" + ReasonWaitingOnExternal // ReasonEphemeralError indicates that the controller encountered an ephemeral error. // An observer should continue to wait. // If this condition persists, the ClusterOperator will eventually enter a degraded state. - ReasonEphemeralError = "EphemeralError" + ReasonEphemeralError // ReasonNonRetryableError indicates that the controller encountered a non-retryable error. - ReasonNonRetryableError = "NonRetryableError" - - // ConditionAvailableSuffix is the suffix added to a controller prefix to - // form the controller's available condition type. - ConditionAvailableSuffix = "Available" - - // ConditionProgressingSuffix is the suffix added to a controller prefix to - // form the controller's progressing condition type. - ConditionProgressingSuffix = "Progressing" + ReasonNonRetryableError ) +// ReasonFromString returns a Reason enum value from a string. It returns +// ReasonUnknown if the string is not a valid Reason. +func ReasonFromString(reason string) Reason { + switch reason { + case ReasonUnknown.String(): + return ReasonUnknown + case ReasonAsExpected.String(): + return ReasonAsExpected + case ReasonUninitialized.String(): + return ReasonUninitialized + case ReasonProgressing.String(): + return ReasonProgressing + case ReasonWaitingOnExternal.String(): + return ReasonWaitingOnExternal + case ReasonEphemeralError.String(): + return ReasonEphemeralError + case ReasonNonRetryableError.String(): + return ReasonNonRetryableError + default: + return ReasonUnknown + } +} + // CAPIFieldOwner returns a qualifiedclient.FieldOwner for the given qualifier. // The qualifier should identify the writer in the context of the CAPI operator, // for example a controller name. @@ -95,6 +140,10 @@ type ReconcileResult struct { // current state if not set explicitly available *partialCondition + // if operatorVersion is set, we will update the ClusterOperator operator + // version to this value when writing status + operatorVersion string + err error requeueAfter time.Duration } @@ -118,6 +167,13 @@ func (r ReconcileResult) withError(err error) ReconcileResult { return r } +// WithUpdateOperatorVersion causes the reconcile result to also update the +// operator version when writing status to the ClusterOperator. +func (r ReconcileResult) WithUpdateOperatorVersion(operatorVersion string) ReconcileResult { + r.operatorVersion = operatorVersion + return r +} + // Result returns a reconcile.Result for controller-runtime. func (r *ReconcileResult) Result() (ctrl.Result, error) { // controller-runtime requires Result{} to be empty when returning an error. @@ -145,8 +201,8 @@ type ControllerResultGenerator string // Success returns a ReconcileResult indicating that the controller has succeeded. // Returning this result will not requeue the controller. func (c ControllerResultGenerator) Success() ReconcileResult { - return newReconcileResult(c, configv1.ConditionFalse, ReasonAsExpected, "Success"). - withAvailable(configv1.ConditionTrue, ReasonAsExpected, "Success") + return newReconcileResult(c, configv1.ConditionFalse, ReasonAsExpected.String(), "Success"). + withAvailable(configv1.ConditionTrue, ReasonAsExpected.String(), "Success") } // SuccessP is a convenience wrapper around Success that returns a pointer to the ReconcileResult. @@ -159,7 +215,7 @@ func (c ControllerResultGenerator) SuccessP() *ReconcileResult { // immediately, for example after writing status to a watched resource. // Returning this result will not requeue the controller directly. func (c ControllerResultGenerator) Progressing(message string) ReconcileResult { - return newReconcileResult(c, configv1.ConditionTrue, ReasonProgressing, message) + return newReconcileResult(c, configv1.ConditionTrue, ReasonProgressing.String(), message) } // ProgressingP is a convenience wrapper around Progressing that returns a pointer to the ReconcileResult. @@ -174,7 +230,7 @@ func (c ControllerResultGenerator) ProgressingP(message string) *ReconcileResult func (c ControllerResultGenerator) WaitingOnExternal(waitDescription string) ReconcileResult { message := fmt.Sprintf("Waiting on %s", waitDescription) - return newReconcileResult(c, configv1.ConditionTrue, ReasonWaitingOnExternal, message) + return newReconcileResult(c, configv1.ConditionTrue, ReasonWaitingOnExternal.String(), message) } // WaitingOnExternalP is a convenience wrapper around WaitingOnExternal that returns a pointer to the ReconcileResult. @@ -190,7 +246,7 @@ func (c ControllerResultGenerator) Error(err error) ReconcileResult { return c.nonRetryableError(err) } - return newReconcileResult(c, configv1.ConditionTrue, ReasonEphemeralError, err.Error()). + return newReconcileResult(c, configv1.ConditionTrue, ReasonEphemeralError.String(), err.Error()). withError(err) } @@ -217,20 +273,21 @@ func (c ControllerResultGenerator) NonRetryableErrorP(err error) *ReconcileResul } func (c ControllerResultGenerator) nonRetryableError(terminalErr error) ReconcileResult { - return newReconcileResult(c, configv1.ConditionFalse, ReasonNonRetryableError, terminalErr.Error()). - withAvailable(configv1.ConditionFalse, ReasonNonRetryableError, terminalErr.Error()). + return newReconcileResult(c, configv1.ConditionFalse, ReasonNonRetryableError.String(), terminalErr.Error()). + withAvailable(configv1.ConditionFalse, ReasonNonRetryableError.String(), terminalErr.Error()). withError(terminalErr) } func (c ControllerResultGenerator) condition(condType string, status configv1.ConditionStatus, reason, message string) *configv1apply.ClusterOperatorStatusConditionApplyConfiguration { return configv1apply.ClusterOperatorStatusCondition(). - WithType(c.conditionType(condType)). + WithType(c.SubConditionType(condType)). WithStatus(status). WithReason(reason). WithMessage(message) } -func (c ControllerResultGenerator) conditionType(condType string) configv1.ClusterStatusConditionType { +// SubConditionType returns the ClusterStatusConditionType for the given condition type. +func (c ControllerResultGenerator) SubConditionType(condType string) configv1.ClusterStatusConditionType { return configv1.ClusterStatusConditionType(string(c) + condType) } @@ -242,6 +299,65 @@ func (r *ReconcileResult) WriteClusterOperatorStatus(ctx context.Context, log lo return fmt.Errorf("failed to get ClusterOperator: %w", err) } + // Extract currently managed fields. This ensures that we preserve operator version if we're not updating it. + clusterOperatorApplyConfig, err := configv1apply.ExtractClusterOperatorStatus(co, string(CAPIFieldOwner(r.ControllerResultGenerator))) + if err != nil { + return fmt.Errorf("failed to extract ClusterOperator apply configuration: %w", err) + } + + clusterOperatorApplyConfig = clusterOperatorApplyConfig.WithUID(co.UID) + + conditions := r.constructPartialConditions(co) + conditionsUpdated := MergeConditions(conditions, co.Status.Conditions) + + releaseVersionNeedsUpdate := false + if r.operatorVersion != "" { + releaseVersionNeedsUpdate = func() bool { + for _, version := range co.Status.Versions { + if version.Name == OperatorVersionKey { + return version.Version != r.operatorVersion + } + } + + return true + }() + } + + if !conditionsUpdated && !releaseVersionNeedsUpdate { + return nil + } + + status := clusterOperatorApplyConfig.Status + if status == nil { + status = configv1apply.ClusterOperatorStatus() + } + + // Clear previously extracted conditions to avoid duplicates, as + // WithConditions appends to the existing slice. + status.Conditions = nil + + status = status.WithConditions(conditions...) + if r.operatorVersion != "" { + // Clear previously extracted versions to avoid duplicates, as + // WithVersions appends to the existing slice. + status.Versions = nil + status = status.WithVersions( + configv1apply.OperandVersion(). + WithName(OperatorVersionKey). + WithVersion(r.operatorVersion)) + } + + patch := util.ApplyConfigPatch(clusterOperatorApplyConfig.WithStatus(status)) + if err := k8sclient.Status().Patch(ctx, co, patch, CAPIFieldOwner(r.ControllerResultGenerator), client.ForceOwnership); err != nil { + return fmt.Errorf("failed to patch ClusterOperator status: %w", err) + } + + return nil +} + +// constructPartialConditions returns a set of condition apply configurations +// for the ReconcileResult. They do not yet have LastTransitionTime set. +func (r *ReconcileResult) constructPartialConditions(co *configv1.ClusterOperator) []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration { // The following behaviours are intended to implement the semantics of // - configv1.ClusterOperatorStatusAvailable // - configv1.ClusterOperatorStatusProgressing @@ -264,7 +380,6 @@ func (r *ReconcileResult) WriteClusterOperatorStatus(ctx context.Context, log lo // administrator's intervention. Currently we set it for non-retryable // errors. Otherwise we copy the previous state of the Available condition // if it exists. - conditions := []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{ r.condition(ConditionProgressingSuffix, r.progressing.status, r.progressing.reason, r.progressing.message), } @@ -275,28 +390,13 @@ func (r *ReconcileResult) WriteClusterOperatorStatus(ctx context.Context, log lo } else { // Infer Available condition from existing state, don't write if not // already present - currentAvailable := findClusterOperatorCondition(r.conditionType(ConditionAvailableSuffix), co.Status.Conditions) + currentAvailable := findClusterOperatorCondition(r.SubConditionType(ConditionAvailableSuffix), co.Status.Conditions) if currentAvailable != nil { conditions = append(conditions, r.condition(ConditionAvailableSuffix, currentAvailable.Status, currentAvailable.Reason, currentAvailable.Message)) } } - updated := mergeConditions(conditions, co.Status.Conditions) - if !updated { - return nil - } - - clusterOperatorApplyConfig := configv1apply.ClusterOperator(ClusterOperatorName). - WithUID(co.UID). - WithStatus(configv1apply.ClusterOperatorStatus(). - WithConditions(conditions...)) - - patch := util.ApplyConfigPatch(clusterOperatorApplyConfig) - if err := k8sclient.Status().Patch(ctx, co, patch, CAPIFieldOwner(r.ControllerResultGenerator), client.ForceOwnership); err != nil { - return fmt.Errorf("failed to patch ClusterOperator status: %w", err) - } - - return nil + return conditions } func findClusterOperatorCondition(condType configv1.ClusterStatusConditionType, conditions []configv1.ClusterOperatorStatusCondition) *configv1.ClusterOperatorStatusCondition { @@ -309,10 +409,10 @@ func findClusterOperatorCondition(condType configv1.ClusterStatusConditionType, return nil } -// mergeConditions sets LastTransitionTime on each new condition based on the +// MergeConditions sets LastTransitionTime on each new condition based on the // existing conditions. If a condition's Status/Reason/Message are unchanged, // LastTransitionTime is preserved from the existing condition. -func mergeConditions(newConditions []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration, existingConditions []configv1.ClusterOperatorStatusCondition) bool { +func MergeConditions(newConditions []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration, existingConditions []configv1.ClusterOperatorStatusCondition) bool { now := metav1.Now() updated := false diff --git a/pkg/operatorstatus/controller_status_test.go b/pkg/operatorstatus/controller_status_test.go index 0b10ef356c..8e8578f624 100644 --- a/pkg/operatorstatus/controller_status_test.go +++ b/pkg/operatorstatus/controller_status_test.go @@ -19,6 +19,7 @@ import ( "context" "errors" "fmt" + "os" "testing" "time" @@ -27,16 +28,96 @@ import ( configv1 "github.com/openshift/api/config/v1" configv1apply "github.com/openshift/client-go/config/applyconfigurations/config/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" ctrl "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/envtest" "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/openshift/cluster-capi-operator/pkg/test" + "github.com/openshift/cluster-capi-operator/pkg/util" +) + +var ( + testEnv *envtest.Environment + cl client.WithWatch ) const testResultGenerator ControllerResultGenerator = "Test" +const defaultReleaseVersion = "1.0.0" + +func TestMain(m *testing.M) { + code, err := runTests(m) + if err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + os.Exit(1) + } + + os.Exit(code) +} + +func runTests(m *testing.M) (_ int, err error) { + testEnv = &envtest.Environment{} + + _, cl, err = test.StartEnvTest(testEnv) + if err != nil { + return 1, fmt.Errorf("failed to start envtest: %w", err) + } + + defer func() { err = errors.Join(err, testEnv.Stop()) }() + + return m.Run(), nil +} + +// createClusterOperator creates a ClusterOperator in the envtest API server +// and optionally sets initial status conditions via a status update. It +// registers a cleanup function to delete the object when the test completes. +func createClusterOperator(t *testing.T, conditions []configv1.ClusterOperatorStatusCondition) *configv1.ClusterOperator { + t.Helper() + g := NewWithT(t) + + co := &configv1.ClusterOperator{ + ObjectMeta: metav1.ObjectMeta{ + Name: ClusterOperatorName, + }, + } + + g.Expect(cl.Create(t.Context(), co)).To(Succeed()) + t.Cleanup(func() { + g.Expect(client.IgnoreNotFound(cl.Delete(context.Background(), co))).To(Succeed()) + }) + + if len(conditions) > 0 { + co.Status.Conditions = conditions + g.Expect(cl.Status().Update(t.Context(), co)).To(Succeed()) + } + + return co +} + +// seedOperatorVersion performs an SSA status patch to set status.versions on +// the ClusterOperator under the given field owner. This establishes field +// ownership naturally through the API server's managed fields tracker. +func seedOperatorVersion(ctx context.Context, k8sClient client.Client, fieldOwner client.FieldOwner) error { + co := &configv1.ClusterOperator{} + if err := k8sClient.Get(ctx, client.ObjectKey{Name: ClusterOperatorName}, co); err != nil { + return err + } + + applyConfig := configv1apply.ClusterOperator(ClusterOperatorName). + WithUID(co.UID). + WithStatus(configv1apply.ClusterOperatorStatus(). + WithVersions( + configv1apply.OperandVersion(). + WithName(OperatorVersionKey). + WithVersion(defaultReleaseVersion), + ), + ) + + patch := util.ApplyConfigPatch(applyConfig) + + return k8sClient.Status().Patch(ctx, co, patch, fieldOwner, client.ForceOwnership) +} func TestSuccess(t *testing.T) { g := NewWithT(t) @@ -44,13 +125,13 @@ func TestSuccess(t *testing.T) { g.Expect(result.progressing).To(Equal(partialCondition{ status: configv1.ConditionFalse, - reason: ReasonAsExpected, + reason: ReasonAsExpected.String(), message: "Success", })) g.Expect(result.available).To(HaveValue(Equal(partialCondition{ status: configv1.ConditionTrue, - reason: ReasonAsExpected, + reason: ReasonAsExpected.String(), message: "Success", }))) @@ -67,7 +148,7 @@ func TestProgressing(t *testing.T) { g.Expect(result.progressing).To(Equal(partialCondition{ status: configv1.ConditionTrue, - reason: ReasonProgressing, + reason: ReasonProgressing.String(), message: "installing components", })) @@ -82,7 +163,7 @@ func TestWaitingOnExternal(t *testing.T) { g.Expect(result.progressing).To(Equal(partialCondition{ status: configv1.ConditionTrue, - reason: ReasonWaitingOnExternal, + reason: ReasonWaitingOnExternal.String(), message: "Waiting on infrastructure", })) @@ -99,7 +180,7 @@ func TestError(t *testing.T) { g.Expect(result.progressing).To(Equal(partialCondition{ status: configv1.ConditionTrue, - reason: ReasonEphemeralError, + reason: ReasonEphemeralError.String(), message: "connection refused", })) @@ -117,13 +198,13 @@ func TestError(t *testing.T) { g.Expect(result.progressing).To(Equal(partialCondition{ status: configv1.ConditionFalse, - reason: ReasonNonRetryableError, + reason: ReasonNonRetryableError.String(), message: termErr.Error(), })) g.Expect(result.available).To(HaveValue(Equal(partialCondition{ status: configv1.ConditionFalse, - reason: ReasonNonRetryableError, + reason: ReasonNonRetryableError.String(), message: termErr.Error(), }))) @@ -139,13 +220,13 @@ func TestNonRetryableError(t *testing.T) { g.Expect(result.progressing).To(Equal(partialCondition{ status: configv1.ConditionFalse, - reason: ReasonNonRetryableError, + reason: ReasonNonRetryableError.String(), message: "terminal error: bad config", })) g.Expect(result.available).To(HaveValue(Equal(partialCondition{ status: configv1.ConditionFalse, - reason: ReasonNonRetryableError, + reason: ReasonNonRetryableError.String(), message: "terminal error: bad config", }))) @@ -160,13 +241,13 @@ func TestNonRetryableError(t *testing.T) { g.Expect(result.progressing).To(Equal(partialCondition{ status: configv1.ConditionFalse, - reason: ReasonNonRetryableError, + reason: ReasonNonRetryableError.String(), message: "terminal error: already wrapped", })) g.Expect(result.available).To(HaveValue(Equal(partialCondition{ status: configv1.ConditionFalse, - reason: ReasonNonRetryableError, + reason: ReasonNonRetryableError.String(), message: "terminal error: already wrapped", }))) @@ -223,7 +304,7 @@ func TestWithRequeueAfter(t *testing.T) { } // applyCondition builds a ClusterOperatorStatusConditionApplyConfiguration for -// use in mergeConditions tests. +// use in MergeConditions tests. func applyCondition(condType configv1.ClusterStatusConditionType, status configv1.ConditionStatus, reason, message string) *configv1apply.ClusterOperatorStatusConditionApplyConfiguration { return configv1apply.ClusterOperatorStatusCondition(). WithType(condType). @@ -291,7 +372,7 @@ func TestMergeConditions(t *testing.T) { } cond := applyCondition(tc.new.condType, tc.new.status, tc.new.reason, tc.new.message) - updated := mergeConditions( + updated := MergeConditions( []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{cond}, existing, ) @@ -312,7 +393,7 @@ func TestMergeConditions(t *testing.T) { cond := applyCondition("Progressing", configv1.ConditionFalse, "AsExpected", "Success") g.Expect(cond.LastTransitionTime).To(BeNil()) - updated := mergeConditions( + updated := MergeConditions( []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{cond}, nil, ) @@ -337,7 +418,7 @@ func TestMergeConditions(t *testing.T) { unchangedCond := applyCondition("Progressing", configv1.ConditionFalse, "AsExpected", "Success") newCond := applyCondition("Degraded", configv1.ConditionFalse, "AsExpected", "Success") - updated := mergeConditions( + updated := MergeConditions( []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{unchangedCond, newCond}, existing, ) @@ -372,7 +453,7 @@ func TestMergeConditions(t *testing.T) { cond1 := applyCondition("Progressing", configv1.ConditionFalse, "AsExpected", "Success") cond2 := applyCondition("Degraded", configv1.ConditionFalse, "AsExpected", "Success") - updated := mergeConditions( + updated := MergeConditions( []*configv1apply.ClusterOperatorStatusConditionApplyConfiguration{cond1, cond2}, existing, ) @@ -383,19 +464,6 @@ func TestMergeConditions(t *testing.T) { }) } -func newFakeClient(objs ...client.Object) client.WithWatch { - scheme := runtime.NewScheme() - if err := configv1.AddToScheme(scheme); err != nil { - panic(err) - } - - return fake.NewClientBuilder(). - WithScheme(scheme). - WithObjects(objs...). - WithStatusSubresource(&configv1.ClusterOperator{}). - Build() -} - func TestWriteClusterOperatorStatus(t *testing.T) { log := testr.New(t) @@ -409,7 +477,7 @@ func TestWriteClusterOperatorStatus(t *testing.T) { { Type: "TestAvailable", Status: configv1.ConditionTrue, - Reason: ReasonAsExpected, + Reason: ReasonAsExpected.String(), Message: "Success", LastTransitionTime: metav1.Now(), }, @@ -425,58 +493,48 @@ func TestWriteClusterOperatorStatus(t *testing.T) { { name: "Success writes Progressing and Available conditions", result: testResultGenerator.Success(), - wantProgressing: expectedCondition{configv1.ConditionFalse, ReasonAsExpected, "Success"}, - wantAvailable: &expectedCondition{configv1.ConditionTrue, ReasonAsExpected, "Success"}, + wantProgressing: expectedCondition{configv1.ConditionFalse, ReasonAsExpected.String(), "Success"}, + wantAvailable: &expectedCondition{configv1.ConditionTrue, ReasonAsExpected.String(), "Success"}, }, { name: "Progressing without existing Available does not write Available", result: testResultGenerator.Progressing("installing components"), - wantProgressing: expectedCondition{configv1.ConditionTrue, ReasonProgressing, "installing components"}, + wantProgressing: expectedCondition{configv1.ConditionTrue, ReasonProgressing.String(), "installing components"}, wantAvailable: nil, }, { name: "Progressing with existing Available preserves Available", existingConditions: existingAvailable, result: testResultGenerator.Progressing("installing components"), - wantProgressing: expectedCondition{configv1.ConditionTrue, ReasonProgressing, "installing components"}, - wantAvailable: &expectedCondition{configv1.ConditionTrue, ReasonAsExpected, "Success"}, + wantProgressing: expectedCondition{configv1.ConditionTrue, ReasonProgressing.String(), "installing components"}, + wantAvailable: &expectedCondition{configv1.ConditionTrue, ReasonAsExpected.String(), "Success"}, }, { name: "Error with existing Available preserves Available", existingConditions: existingAvailable, result: testResultGenerator.Error(fmt.Errorf("connection refused")), - wantProgressing: expectedCondition{configv1.ConditionTrue, ReasonEphemeralError, "connection refused"}, - wantAvailable: &expectedCondition{configv1.ConditionTrue, ReasonAsExpected, "Success"}, + wantProgressing: expectedCondition{configv1.ConditionTrue, ReasonEphemeralError.String(), "connection refused"}, + wantAvailable: &expectedCondition{configv1.ConditionTrue, ReasonAsExpected.String(), "Success"}, }, { name: "WaitingOnExternal with existing Available preserves Available", existingConditions: existingAvailable, result: testResultGenerator.WaitingOnExternal("infrastructure"), - wantProgressing: expectedCondition{configv1.ConditionTrue, ReasonWaitingOnExternal, "Waiting on infrastructure"}, - wantAvailable: &expectedCondition{configv1.ConditionTrue, ReasonAsExpected, "Success"}, + wantProgressing: expectedCondition{configv1.ConditionTrue, ReasonWaitingOnExternal.String(), "Waiting on infrastructure"}, + wantAvailable: &expectedCondition{configv1.ConditionTrue, ReasonAsExpected.String(), "Success"}, }, { name: "NonRetryableError explicitly sets Available=False", existingConditions: existingAvailable, result: testResultGenerator.NonRetryableError(fmt.Errorf("bad config")), - wantProgressing: expectedCondition{configv1.ConditionFalse, ReasonNonRetryableError, "terminal error: bad config"}, - wantAvailable: &expectedCondition{configv1.ConditionFalse, ReasonNonRetryableError, "terminal error: bad config"}, + wantProgressing: expectedCondition{configv1.ConditionFalse, ReasonNonRetryableError.String(), "terminal error: bad config"}, + wantAvailable: &expectedCondition{configv1.ConditionFalse, ReasonNonRetryableError.String(), "terminal error: bad config"}, }, } { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - co := &configv1.ClusterOperator{ - ObjectMeta: metav1.ObjectMeta{ - Name: ClusterOperatorName, - UID: types.UID("test-uid"), - }, - Status: configv1.ClusterOperatorStatus{ - Conditions: tc.existingConditions, - }, - } - - cl := newFakeClient(co) + co := createClusterOperator(t, tc.existingConditions) result := tc.result err := result.WriteClusterOperatorStatus(t.Context(), log, cl) @@ -505,33 +563,25 @@ func TestWriteClusterOperatorStatus(t *testing.T) { t.Run("skips patch when conditions unchanged", func(t *testing.T) { g := NewWithT(t) - co := &configv1.ClusterOperator{ - ObjectMeta: metav1.ObjectMeta{ - Name: ClusterOperatorName, - UID: types.UID("test-uid"), + createClusterOperator(t, []configv1.ClusterOperatorStatusCondition{ + { + Type: "TestProgressing", + Status: configv1.ConditionFalse, + Reason: ReasonAsExpected.String(), + Message: "Success", + LastTransitionTime: metav1.Now(), }, - Status: configv1.ClusterOperatorStatus{ - Conditions: []configv1.ClusterOperatorStatusCondition{ - { - Type: "TestProgressing", - Status: configv1.ConditionFalse, - Reason: ReasonAsExpected, - Message: "Success", - LastTransitionTime: metav1.Now(), - }, - { - Type: "TestAvailable", - Status: configv1.ConditionTrue, - Reason: ReasonAsExpected, - Message: "Success", - LastTransitionTime: metav1.Now(), - }, - }, + { + Type: "TestAvailable", + Status: configv1.ConditionTrue, + Reason: ReasonAsExpected.String(), + Message: "Success", + LastTransitionTime: metav1.Now(), }, - } + }) patchCalled := false - cl := interceptor.NewClient(newFakeClient(co), interceptor.Funcs{ + interceptCl := interceptor.NewClient(cl, interceptor.Funcs{ SubResourcePatch: func(ctx context.Context, c client.Client, subResourceName string, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error { patchCalled = true return c.SubResource(subResourceName).Patch(ctx, obj, patch, opts...) @@ -539,7 +589,7 @@ func TestWriteClusterOperatorStatus(t *testing.T) { }) result := testResultGenerator.Success() - err := result.WriteClusterOperatorStatus(t.Context(), log, cl) + err := result.WriteClusterOperatorStatus(t.Context(), log, interceptCl) g.Expect(err).ToNot(HaveOccurred()) g.Expect(patchCalled).To(BeFalse()) }) @@ -547,33 +597,25 @@ func TestWriteClusterOperatorStatus(t *testing.T) { t.Run("patches when conditions changed", func(t *testing.T) { g := NewWithT(t) - co := &configv1.ClusterOperator{ - ObjectMeta: metav1.ObjectMeta{ - Name: ClusterOperatorName, - UID: types.UID("test-uid"), + createClusterOperator(t, []configv1.ClusterOperatorStatusCondition{ + { + Type: "TestProgressing", + Status: configv1.ConditionTrue, + Reason: ReasonProgressing.String(), + Message: "installing components", + LastTransitionTime: metav1.Now(), }, - Status: configv1.ClusterOperatorStatus{ - Conditions: []configv1.ClusterOperatorStatusCondition{ - { - Type: "TestProgressing", - Status: configv1.ConditionTrue, - Reason: ReasonProgressing, - Message: "installing components", - LastTransitionTime: metav1.Now(), - }, - { - Type: "TestAvailable", - Status: configv1.ConditionTrue, - Reason: ReasonAsExpected, - Message: "Success", - LastTransitionTime: metav1.Now(), - }, - }, + { + Type: "TestAvailable", + Status: configv1.ConditionTrue, + Reason: ReasonAsExpected.String(), + Message: "Success", + LastTransitionTime: metav1.Now(), }, - } + }) patchCalled := false - cl := interceptor.NewClient(newFakeClient(co), interceptor.Funcs{ + interceptCl := interceptor.NewClient(cl, interceptor.Funcs{ SubResourcePatch: func(ctx context.Context, c client.Client, subResourceName string, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error { patchCalled = true return c.SubResource(subResourceName).Patch(ctx, obj, patch, opts...) @@ -581,7 +623,7 @@ func TestWriteClusterOperatorStatus(t *testing.T) { }) result := testResultGenerator.Success() - err := result.WriteClusterOperatorStatus(t.Context(), log, cl) + err := result.WriteClusterOperatorStatus(t.Context(), log, interceptCl) g.Expect(err).ToNot(HaveOccurred()) g.Expect(patchCalled).To(BeTrue()) }) @@ -589,14 +631,88 @@ func TestWriteClusterOperatorStatus(t *testing.T) { t.Run("returns error when ClusterOperator not found", func(t *testing.T) { g := NewWithT(t) - // No ClusterOperator seeded - cl := newFakeClient() - + // No ClusterOperator created. result := testResultGenerator.Success() err := result.WriteClusterOperatorStatus(t.Context(), log, cl) g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring("failed to get ClusterOperator")) }) + + t.Run("does not update versions with a different field owner when WithUpdateOperatorVersion is not set", func(t *testing.T) { + g := NewWithT(t) + + co := createClusterOperator(t, nil) + + // Seed a version under a different field owner. + g.Expect(seedOperatorVersion(t.Context(), cl, client.FieldOwner("other-owner"))).To(Succeed()) + + // Write status without WithUpdateOperatorVersion. + result := testResultGenerator.Success() + g.Expect(result.WriteClusterOperatorStatus(t.Context(), log, cl)).To(Succeed()) + + g.Expect(cl.Get(t.Context(), client.ObjectKeyFromObject(co), co)).To(Succeed()) + g.Expect(co.Status.Versions).To(ContainElement(SatisfyAll( + HaveField("Name", Equal(OperatorVersionKey)), + HaveField("Version", Equal(defaultReleaseVersion)), + ))) + }) + + t.Run("does not update versions with same field owner when WithUpdateOperatorVersion is not set", func(t *testing.T) { + g := NewWithT(t) + + co := createClusterOperator(t, nil) + + // Seed a version under the same field owner as the code under test. + g.Expect(seedOperatorVersion(t.Context(), cl, CAPIFieldOwner(testResultGenerator))).To(Succeed()) + + // Write status without WithUpdateOperatorVersion. + result := testResultGenerator.Success() + g.Expect(result.WriteClusterOperatorStatus(t.Context(), log, cl)).To(Succeed()) + + g.Expect(cl.Get(t.Context(), client.ObjectKeyFromObject(co), co)).To(Succeed()) + g.Expect(co.Status.Versions).To(ContainElement(SatisfyAll( + HaveField("Name", Equal(OperatorVersionKey)), + HaveField("Version", Equal(defaultReleaseVersion)), + ))) + }) + + t.Run("overwrites versions with a different field owner when WithUpdateOperatorVersion is set", func(t *testing.T) { + g := NewWithT(t) + + co := createClusterOperator(t, nil) + + // Seed a version under a different field owner. + g.Expect(seedOperatorVersion(t.Context(), cl, client.FieldOwner("other-owner"))).To(Succeed()) + + // Write status with WithUpdateOperatorVersion to a new version. + result := testResultGenerator.Success().WithUpdateOperatorVersion("2.0.0") + g.Expect(result.WriteClusterOperatorStatus(t.Context(), log, cl)).To(Succeed()) + + g.Expect(cl.Get(t.Context(), client.ObjectKeyFromObject(co), co)).To(Succeed()) + g.Expect(co.Status.Versions).To(ContainElement(SatisfyAll( + HaveField("Name", Equal(OperatorVersionKey)), + HaveField("Version", Equal("2.0.0")), + ))) + }) + + t.Run("overwrites versions with same field owner when WithUpdateOperatorVersion is set", func(t *testing.T) { + g := NewWithT(t) + + co := createClusterOperator(t, nil) + + // Seed a version under the same field owner as the code under test. + g.Expect(seedOperatorVersion(t.Context(), cl, CAPIFieldOwner(testResultGenerator))).To(Succeed()) + + // Write status with WithUpdateOperatorVersion to a new version. + result := testResultGenerator.Success().WithUpdateOperatorVersion("2.0.0") + g.Expect(result.WriteClusterOperatorStatus(t.Context(), log, cl)).To(Succeed()) + + g.Expect(cl.Get(t.Context(), client.ObjectKeyFromObject(co), co)).To(Succeed()) + g.Expect(co.Status.Versions).To(ConsistOf(SatisfyAll( + HaveField("Name", Equal(OperatorVersionKey)), + HaveField("Version", Equal("2.0.0")), + ))) + }) } func TestFindClusterOperatorCondition(t *testing.T) { diff --git a/pkg/operatorstatus/operator_status.go b/pkg/operatorstatus/operator_status.go index 06932b114a..5ecb19ea93 100644 --- a/pkg/operatorstatus/operator_status.go +++ b/pkg/operatorstatus/operator_status.go @@ -33,11 +33,6 @@ import ( "github.com/openshift/library-go/pkg/config/clusteroperator/v1helpers" ) -const ( - // ReasonSyncFailed is the reason for the condition when the operator failed to sync resources. - ReasonSyncFailed = "SyncingFailed" -) - // ClusterOperatorStatusClient is a client for managing the status of the ClusterOperator object. type ClusterOperatorStatusClient struct { client.Client @@ -90,9 +85,8 @@ func (r *ClusterOperatorStatusClient) SetStatusDegraded(ctx context.Context, rec message := fmt.Sprintf("Failed to resync because %v", reconcileErr) conds := []configv1.ClusterOperatorStatusCondition{ - NewClusterOperatorStatusCondition(configv1.OperatorDegraded, configv1.ConditionTrue, - ReasonSyncFailed, message), - NewClusterOperatorStatusCondition(configv1.OperatorUpgradeable, configv1.ConditionFalse, ReasonAsExpected, ""), + NewClusterOperatorStatusCondition(configv1.OperatorDegraded, configv1.ConditionTrue, ReasonEphemeralError, message), + NewClusterOperatorStatusCondition(configv1.OperatorUpgradeable, configv1.ConditionFalse, ReasonAsExpected, message), } r.Recorder.Eventf(co, corev1.EventTypeWarning, "Status degraded", reconcileErr.Error()) @@ -191,20 +185,15 @@ func (r *ClusterOperatorStatusClient) SyncStatus(ctx context.Context, co *config return nil } -// OperandVersions returns the operand versions for the ClusterOperator. -func (r *ClusterOperatorStatusClient) OperandVersions() []configv1.OperandVersion { - return []configv1.OperandVersion{{Name: controllers.OperatorVersionKey, Version: r.ReleaseVersion}} -} - // NewClusterOperatorStatusCondition creates a new ClusterOperatorStatusCondition. func NewClusterOperatorStatusCondition(conditionType configv1.ClusterStatusConditionType, - conditionStatus configv1.ConditionStatus, reason string, + conditionStatus configv1.ConditionStatus, reason Reason, message string) configv1.ClusterOperatorStatusCondition { return configv1.ClusterOperatorStatusCondition{ Type: conditionType, Status: conditionStatus, LastTransitionTime: metav1.Now(), - Reason: reason, + Reason: reason.String(), Message: message, } } diff --git a/pkg/operatorstatus/reason_string.go b/pkg/operatorstatus/reason_string.go new file mode 100644 index 0000000000..a59938248f --- /dev/null +++ b/pkg/operatorstatus/reason_string.go @@ -0,0 +1,30 @@ +// Code generated by "stringer -type=Reason -trimprefix=Reason"; DO NOT EDIT. + +package operatorstatus + +import "strconv" + +func _() { + // An "invalid array index" compiler error signifies that the constant values have changed. + // Re-run the stringer command to generate them again. + var x [1]struct{} + _ = x[ReasonUnknown-0] + _ = x[ReasonAsExpected-1] + _ = x[ReasonUninitialized-2] + _ = x[ReasonProgressing-3] + _ = x[ReasonWaitingOnExternal-4] + _ = x[ReasonEphemeralError-5] + _ = x[ReasonNonRetryableError-6] +} + +const _Reason_name = "UnknownAsExpectedUninitializedProgressingWaitingOnExternalEphemeralErrorNonRetryableError" + +var _Reason_index = [...]uint8{0, 7, 17, 30, 41, 58, 72, 89} + +func (i Reason) String() string { + idx := int(i) - 0 + if i < 0 || idx >= len(_Reason_index)-1 { + return "Reason(" + strconv.FormatInt(int64(i), 10) + ")" + } + return _Reason_name[_Reason_index[idx]:_Reason_index[idx+1]] +} diff --git a/pkg/operatorstatus/watch_predicates.go b/pkg/operatorstatus/watch_predicates.go index 894e08b833..2b54e73b10 100644 --- a/pkg/operatorstatus/watch_predicates.go +++ b/pkg/operatorstatus/watch_predicates.go @@ -18,6 +18,7 @@ package operatorstatus import ( "context" + "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" @@ -49,6 +50,33 @@ func ClusterOperatorOnceOnly() predicate.Funcs { } } +// ClusterOperatorStatusChanged returns a predicate that triggers on create +// events for the cluster-api ClusterOperator, and on update events when +// status.Conditions or status.Versions has changed. +func ClusterOperatorStatusChanged() predicate.Funcs { + isClusterOperator := func(obj runtime.Object) bool { + clusterOperator, ok := obj.(*configv1.ClusterOperator) + return ok && clusterOperator.GetName() == controllers.ClusterOperatorName + } + + return predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { return isClusterOperator(e.Object) }, + UpdateFunc: func(e event.UpdateEvent) bool { + if !isClusterOperator(e.ObjectNew) { + return false + } + + oldCO, _ := e.ObjectOld.(*configv1.ClusterOperator) + newCO, _ := e.ObjectNew.(*configv1.ClusterOperator) + + return !equality.Semantic.DeepEqual(oldCO.Status.Conditions, newCO.Status.Conditions) || + !equality.Semantic.DeepEqual(oldCO.Status.Versions, newCO.Status.Versions) + }, + DeleteFunc: func(e event.DeleteEvent) bool { return false }, + GenericFunc: func(e event.GenericEvent) bool { return false }, + } +} + // ToClusterOperator unconditionally returns a reconcile request for the cluster-api ClusterOperator. func ToClusterOperator(_ context.Context, _ client.Object) []reconcile.Request { return []reconcile.Request{{ diff --git a/pkg/providerimages/configmap.go b/pkg/providerimages/configmap.go new file mode 100644 index 0000000000..adf608f5d7 --- /dev/null +++ b/pkg/providerimages/configmap.go @@ -0,0 +1,45 @@ +/* +Copyright 2026 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package providerimages + +import ( + "errors" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/sets" +) + +var errConfigMapNil = errors.New("ConfigMap cannot be nil") + +// ConfigMapName is the name of the ConfigMap containing current-release provider image references. +const ConfigMapName = "capi-installer-images" + +// ImageRefsFromConfigMap extracts provider image references from a ConfigMap. +// The ConfigMap data values are image references; the keys are discarded. +// Returns an error if the ConfigMap is nil. +func ImageRefsFromConfigMap(cm *corev1.ConfigMap) (sets.Set[string], error) { + if cm == nil { + return nil, errConfigMapNil + } + + result := sets.New[string]() + for _, v := range cm.Data { + result.Insert(v) + } + + return result, nil +} diff --git a/pkg/providerimages/configmap_test.go b/pkg/providerimages/configmap_test.go new file mode 100644 index 0000000000..3f816c58ee --- /dev/null +++ b/pkg/providerimages/configmap_test.go @@ -0,0 +1,68 @@ +/* +Copyright 2026 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package providerimages + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" +) + +var _ = Describe("ImageRefsFromConfigMap", func() { + It("should return a map of provider names to image refs from ConfigMap data", func() { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "capi-installer-images", + Namespace: "openshift-cluster-api-operator", + }, + Data: map[string]string{ + "aws-cluster-api-controllers": "registry/aws@sha256:abc", + "gcp-cluster-api-controllers": "registry/gcp@sha256:def", + }, + } + + result, err := ImageRefsFromConfigMap(cm) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(sets.New[string]( + "registry/aws@sha256:abc", + "registry/gcp@sha256:def", + ))) + }) + + It("should return an empty map when ConfigMap data is empty", func() { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "capi-installer-images", + Namespace: "openshift-cluster-api-operator", + }, + Data: map[string]string{}, + } + + result, err := ImageRefsFromConfigMap(cm) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeEmpty()) + }) + + It("should return an error when ConfigMap is nil", func() { + result, err := ImageRefsFromConfigMap(nil) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(errConfigMapNil)) + Expect(result).To(BeNil()) + }) +}) diff --git a/pkg/providerimages/providerimages.go b/pkg/providerimages/providerimages.go index 4ea83fbd31..e17d7b0fc7 100644 --- a/pkg/providerimages/providerimages.go +++ b/pkg/providerimages/providerimages.go @@ -16,14 +16,18 @@ limitations under the License. package providerimages import ( + "crypto/sha256" + "encoding/hex" "errors" "fmt" "os" "path/filepath" + "strings" "github.com/go-logr/logr" "github.com/openshift/cluster-capi-operator/manifests-gen/providermetadata" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/yaml" ) @@ -31,6 +35,8 @@ const ( metadataFile = "metadata.yaml" manifestsFile = "manifests.yaml" capiOperatorManifestsDir = "capi-operator-manifests" + // ProviderImageMountBase is the base path where provider image volumes are mounted. + ProviderImageMountBase = "/var/lib/provider-images" // AttributeKeyType is the key for the provider type attribute. AttributeKeyType = providermetadata.AttributeKeyType @@ -55,28 +61,32 @@ var ( errNoCapiManifests = errors.New("no capi-manifests directory found") errMissingMetadata = errors.New("missing metadata.yaml in /capi-operator-manifests") errMissingManifests = errors.New("missing manifests.yaml in /capi-operator-manifests") - errImageRefNotFound = errors.New("image ref not found for provider") errContainerNotFound = errors.New("container not found in pod spec") ) // ScanProviderImages scans providerImageDir for subdirectories containing // provider profiles (metadata.yaml + manifests.yaml). imageRefMap maps -// subdirectory names to image references (built from the pod spec). +// expected subdirectory names to image references. func ScanProviderImages(logger logr.Logger, providerImageDir string, imageRefMap map[string]string) ([]ProviderImageManifests, error) { - entries, err := os.ReadDir(providerImageDir) - if err != nil { - return nil, fmt.Errorf("failed to read provider image directory %s: %w", providerImageDir, err) - } - var result []ProviderImageManifests - for _, entry := range entries { - if !entry.IsDir() { - continue + for _, subdir := range sets.List(sets.KeySet(imageRefMap)) { + subdirPath := filepath.Join(providerImageDir, subdir) + + info, err := os.Stat(subdirPath) + if err != nil { + if os.IsNotExist(err) { + logger.Info("Skipping provider directory: expected directory does not exist", "directory", subdir) + continue + } + + return nil, fmt.Errorf("failed to stat provider image directory %s: %w", subdirPath, err) } - subdir := entry.Name() - subdirPath := filepath.Join(providerImageDir, subdir) + if !info.IsDir() { + logger.Info("Skipping provider directory: expected path is not a directory", "directory", subdir) + continue + } profiles, err := discoverProfiles(subdirPath) if err != nil { @@ -91,12 +101,6 @@ func ScanProviderImages(logger logr.Logger, providerImageDir string, imageRefMap imageRef := imageRefMap[subdir] for _, profile := range profiles { - // If the provider has profiles but no image ref, return an error - // instead of a provider with an empty image ref. - if imageRef == "" { - return nil, fmt.Errorf("%w: %s", errImageRefNotFound, subdir) - } - manifestsPath := filepath.Join(subdirPath, capiOperatorManifestsDir, profile.Profile, manifestsFile) result = append(result, ProviderImageManifests{ @@ -111,6 +115,18 @@ func ScanProviderImages(logger logr.Logger, providerImageDir string, imageRefMap return result, nil } +// BuildImageRefMapFromRefs builds a mapping from expected mount subdirectory +// names to image references. +func BuildImageRefMapFromRefs(imageRefs sets.Set[string]) map[string]string { + imageRefMap := make(map[string]string, imageRefs.Len()) + + for _, imageRef := range sets.List(imageRefs) { + imageRefMap[VolumeNameForImageRef(imageRef)] = imageRef + } + + return imageRefMap +} + // BuildImageRefMap builds a mapping from mount subdirectory names to image // references by correlating image volumes with their volume mounts for the // named container in the given PodSpec. @@ -146,6 +162,39 @@ func BuildImageRefMap(podSpec corev1.PodSpec, containerName string) (map[string] return nil, fmt.Errorf("container %q: %w", containerName, errContainerNotFound) } +// VolumeNameForImageRef generates a deterministic, DNS-label-safe volume name +// from an image reference. The volume name consists of a prefix derived from +// the image name and a short hash of the full image reference. +func VolumeNameForImageRef(imageRef string) string { + parts := strings.Split(imageRef, "@") + if len(parts) == 0 { + parts = []string{imageRef} + } + + pathParts := strings.Split(parts[0], "/") + imageName := pathParts[len(pathParts)-1] + + imageName = strings.ToLower(imageName) + imageName = strings.Map(func(r rune) rune { + if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '-' { + return r + } + + return '-' + }, imageName) + + hash := sha256.Sum256([]byte(imageRef)) + shortHash := hex.EncodeToString(hash[:])[:8] + + volumeName := fmt.Sprintf("%s-%s", imageName, shortHash) + + if len(volumeName) > 0 && (volumeName[0] < 'a' || volumeName[0] > 'z') && (volumeName[0] < '0' || volumeName[0] > '9') { + volumeName = "img-" + volumeName + } + + return volumeName +} + // profileManifests holds parsed metadata and manifest content for a single profile. type profileManifests struct { Profile string diff --git a/pkg/providerimages/providerimages_test.go b/pkg/providerimages/providerimages_test.go index e65290365d..c98f7fb148 100644 --- a/pkg/providerimages/providerimages_test.go +++ b/pkg/providerimages/providerimages_test.go @@ -23,9 +23,8 @@ import ( "github.com/go-logr/logr/testr" . "github.com/onsi/gomega" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - "sigs.k8s.io/yaml" + "k8s.io/apimachinery/pkg/util/sets" ) // createMetadataYAML generates valid metadata.yaml content. @@ -163,6 +162,17 @@ func Test_BuildImageRefMap(t *testing.T) { containerName: "my-container", expected: map[string]string{}, }, + { + name: "missing container returns error", + podSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "different-container", + }}, + }, + containerName: "my-container", + wantErr: true, + errContains: `container "my-container": container not found in pod spec`, + }, } for _, tt := range tests { @@ -173,10 +183,7 @@ func Test_BuildImageRefMap(t *testing.T) { if tt.wantErr { g.Expect(err).To(HaveOccurred()) - - if tt.errContains != "" { - g.Expect(err.Error()).To(ContainSubstring(tt.errContains)) - } + g.Expect(err.Error()).To(ContainSubstring(tt.errContains)) return } @@ -187,29 +194,34 @@ func Test_BuildImageRefMap(t *testing.T) { } } -func Test_BuildImageRefMap_DeploymentManifest(t *testing.T) { +func Test_VolumeNameForImageRef(t *testing.T) { g := NewWithT(t) - data, err := os.ReadFile("../../manifests/0000_30_cluster-api-installer_05_deployment.yaml") - g.Expect(err).NotTo(HaveOccurred()) - - var deployment appsv1.Deployment - g.Expect(yaml.Unmarshal(data, &deployment)).To(Succeed()) - - imageRefMap, err := BuildImageRefMap(deployment.Spec.Template.Spec, "capi-operator") - g.Expect(err).NotTo(HaveOccurred()) - - g.Expect(imageRefMap).To(Equal(map[string]string{ - "aws-cluster-api-controllers": "registry.ci.openshift.org/openshift:aws-cluster-api-controllers", - "azure-cluster-api-controllers": "registry.ci.openshift.org/openshift:azure-cluster-api-controllers", - "baremetal-cluster-api-controllers": "registry.ci.openshift.org/openshift:baremetal-cluster-api-controllers", - "cluster-capi-controllers": "registry.ci.openshift.org/openshift:cluster-capi-controllers", - "cluster-capi-operator": "registry.ci.openshift.org/openshift:cluster-capi-operator", - "gcp-cluster-api-controllers": "registry.ci.openshift.org/openshift:gcp-cluster-api-controllers", - "ibmcloud-cluster-api-controllers": "registry.ci.openshift.org/openshift:ibmcloud-cluster-api-controllers", - "openstack-cluster-api-controllers": "registry.ci.openshift.org/openshift:openstack-cluster-api-controllers", - "openstack-resource-controller": "registry.ci.openshift.org/openshift:openstack-resource-controller", - "vsphere-cluster-api-controllers": "registry.ci.openshift.org/openshift:vsphere-cluster-api-controllers", + name := VolumeNameForImageRef("registry.example.com/My.Provider_AWS@sha256:abc123") + g.Expect(name).To(MatchRegexp(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`), "expected a DNS-label-safe volume name") + + g.Expect(VolumeNameForImageRef("registry.example.com/core@sha256:def456")).To( + Equal(VolumeNameForImageRef("registry.example.com/core@sha256:def456")), + "expected the same image ref to always generate the same volume name", + ) + + g.Expect(VolumeNameForImageRef("registry.example.com/aws@sha256:abc")).NotTo( + Equal(VolumeNameForImageRef("registry.example.com/gcp@sha256:def")), + "expected different image refs to generate different volume names", + ) +} + +func Test_BuildImageRefMapFromRefs(t *testing.T) { + g := NewWithT(t) + + imageRefs := sets.New( + "registry.example.com/aws@sha256:abc", + "registry.example.com/gcp@sha256:def", + ) + + g.Expect(BuildImageRefMapFromRefs(imageRefs)).To(Equal(map[string]string{ + VolumeNameForImageRef("registry.example.com/aws@sha256:abc"): "registry.example.com/aws@sha256:abc", + VolumeNameForImageRef("registry.example.com/gcp@sha256:def"): "registry.example.com/gcp@sha256:def", })) } @@ -262,7 +274,9 @@ func Test_ScanProviderImages(t *testing.T) { t.Fatalf("failed to create directory: %v", err) } }, - imageRefMap: map[string]string{}, + imageRefMap: map[string]string{ + "empty-provider": "registry.example.com/empty-provider:v1.0.0", + }, validate: func(t *testing.T, g Gomega, result []ProviderImageManifests) { t.Helper() g.Expect(result).To(BeEmpty()) @@ -285,7 +299,9 @@ func Test_ScanProviderImages(t *testing.T) { t.Fatalf("failed to write file: %v", err) } }, - imageRefMap: map[string]string{}, + imageRefMap: map[string]string{ + "no-manifests-provider": "registry.example.com/no-manifests-provider:v1.0.0", + }, validate: func(t *testing.T, g Gomega, result []ProviderImageManifests) { t.Helper() g.Expect(result).To(BeEmpty()) @@ -300,7 +316,9 @@ func Test_ScanProviderImages(t *testing.T) { "apiVersion: v1\nkind: ConfigMap\n", ) }, - imageRefMap: map[string]string{}, + imageRefMap: map[string]string{ + "bad-provider": "registry.example.com/bad-provider:v1.0.0", + }, wantErr: true, errContains: "missing metadata.yaml", }, @@ -313,7 +331,9 @@ func Test_ScanProviderImages(t *testing.T) { "", // no manifests ) }, - imageRefMap: map[string]string{}, + imageRefMap: map[string]string{ + "bad-provider": "registry.example.com/bad-provider:v1.0.0", + }, wantErr: true, errContains: "missing manifests.yaml", }, @@ -326,7 +346,9 @@ func Test_ScanProviderImages(t *testing.T) { "apiVersion: v1\nkind: ConfigMap\n", ) }, - imageRefMap: map[string]string{}, + imageRefMap: map[string]string{ + "bad-provider": "registry.example.com/bad-provider:v1.0.0", + }, wantErr: true, errContains: "failed to parse metadata.yaml", }, @@ -465,7 +487,7 @@ func Test_ScanProviderImages(t *testing.T) { }, }, { - name: "missing image ref in map returns error", + name: "extra directories on disk not in map are ignored", setup: func(t *testing.T, dir string) { t.Helper() writeProfile(t, dir, "unknown-provider", "default", @@ -474,8 +496,23 @@ func Test_ScanProviderImages(t *testing.T) { ) }, imageRefMap: map[string]string{}, - wantErr: true, - errContains: "image ref not found for provider: unknown-provider", + validate: func(t *testing.T, g Gomega, result []ProviderImageManifests) { + t.Helper() + g.Expect(result).To(BeEmpty()) + }, + }, + { + name: "directory in map that does not exist on disk is skipped", + setup: func(t *testing.T, dir string) { + t.Helper() + }, + imageRefMap: map[string]string{ + "missing-provider": "registry.example.com/missing-provider:v1.0.0", + }, + validate: func(t *testing.T, g Gomega, result []ProviderImageManifests) { + t.Helper() + g.Expect(result).To(BeEmpty()) + }, }, } diff --git a/pkg/providerimages/revision_images.go b/pkg/providerimages/revision_images.go new file mode 100644 index 0000000000..e2c8efcad5 --- /dev/null +++ b/pkg/providerimages/revision_images.go @@ -0,0 +1,38 @@ +/* +Copyright 2026 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package providerimages + +import ( + operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1" + "k8s.io/apimachinery/pkg/util/sets" +) + +// ImageRefsFromRevisions extracts unique image references from ClusterAPI revisions +// and returns a set of deduplicated image references. +func ImageRefsFromRevisions(revisions []operatorv1alpha1.ClusterAPIInstallerRevision) sets.Set[string] { + result := sets.New[string]() + + for _, revision := range revisions { + for _, component := range revision.Components { + if component.Type == operatorv1alpha1.InstallerComponentTypeImage { + result.Insert(string(component.Image.Ref)) + } + } + } + + return result +} diff --git a/pkg/providerimages/revision_images_test.go b/pkg/providerimages/revision_images_test.go new file mode 100644 index 0000000000..5a27a68750 --- /dev/null +++ b/pkg/providerimages/revision_images_test.go @@ -0,0 +1,117 @@ +/* +Copyright 2026 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package providerimages + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1" + "k8s.io/apimachinery/pkg/util/sets" +) + +var _ = Describe("ImageRefsFromRevisions", func() { + It("should return a set of image refs from distinct component images", func() { + revisions := []operatorv1alpha1.ClusterAPIInstallerRevision{ + { + Components: []operatorv1alpha1.ClusterAPIInstallerComponent{ + { + Name: "core", + ClusterAPIInstallerComponentSource: operatorv1alpha1.ClusterAPIInstallerComponentSource{ + Type: operatorv1alpha1.InstallerComponentTypeImage, + Image: operatorv1alpha1.ClusterAPIInstallerComponentImage{ + Ref: "registry/core@sha256:abc123", + }, + }, + }, + { + Name: "aws-infrastructure", + ClusterAPIInstallerComponentSource: operatorv1alpha1.ClusterAPIInstallerComponentSource{ + Type: operatorv1alpha1.InstallerComponentTypeImage, + Image: operatorv1alpha1.ClusterAPIInstallerComponentImage{ + Ref: "registry/aws@sha256:def456", + }, + }, + }, + }, + }, + { + Components: []operatorv1alpha1.ClusterAPIInstallerComponent{ + { + Name: "gcp-infrastructure", + ClusterAPIInstallerComponentSource: operatorv1alpha1.ClusterAPIInstallerComponentSource{ + Type: operatorv1alpha1.InstallerComponentTypeImage, + Image: operatorv1alpha1.ClusterAPIInstallerComponentImage{ + Ref: "registry/gcp@sha256:789abc", + }, + }, + }, + }, + }, + } + + result := ImageRefsFromRevisions(revisions) + Expect(result).To(Equal(sets.New[string]( + "registry/core@sha256:abc123", + "registry/aws@sha256:def456", + "registry/gcp@sha256:789abc", + ))) + }) + + It("should deduplicate overlapping image refs", func() { + revisions := []operatorv1alpha1.ClusterAPIInstallerRevision{ + { + Components: []operatorv1alpha1.ClusterAPIInstallerComponent{ + { + Name: "core", + ClusterAPIInstallerComponentSource: operatorv1alpha1.ClusterAPIInstallerComponentSource{ + Type: operatorv1alpha1.InstallerComponentTypeImage, + Image: operatorv1alpha1.ClusterAPIInstallerComponentImage{ + Ref: "registry/core@sha256:abc123", + }, + }, + }, + }, + }, + { + Components: []operatorv1alpha1.ClusterAPIInstallerComponent{ + { + Name: "core", + ClusterAPIInstallerComponentSource: operatorv1alpha1.ClusterAPIInstallerComponentSource{ + Type: operatorv1alpha1.InstallerComponentTypeImage, + Image: operatorv1alpha1.ClusterAPIInstallerComponentImage{ + Ref: "registry/core@sha256:abc123", + }, + }, + }, + }, + }, + } + + result := ImageRefsFromRevisions(revisions) + Expect(result).To(Equal(sets.New[string]("registry/core@sha256:abc123"))) + }) + + It("should return an empty set for empty revisions slice", func() { + result := ImageRefsFromRevisions([]operatorv1alpha1.ClusterAPIInstallerRevision{}) + Expect(result).To(BeEmpty()) + }) + + It("should return an empty set for nil revisions slice", func() { + result := ImageRefsFromRevisions(nil) + Expect(result).To(BeEmpty()) + }) +}) diff --git a/pkg/test/conditions.go b/pkg/test/conditions.go index f2b67f8c34..1a19df9424 100644 --- a/pkg/test/conditions.go +++ b/pkg/test/conditions.go @@ -331,7 +331,14 @@ func toMatcher(v interface{}) types.GomegaMatcher { return matcher } - return gomega.Equal(v) + // Convert fmt.Stringer values (e.g. operatorstatus.Reason) to their + // string representation so that comparisons against string-typed + // condition fields succeed. + if s, ok := v.(fmt.Stringer); ok { + return gomega.BeEquivalentTo(s.String()) + } + + return gomega.BeEquivalentTo(v) } // getStringValue converts a reflect.Value to its string representation. diff --git a/vendor/golang.org/x/tools/cmd/stringer/stringer.go b/vendor/golang.org/x/tools/cmd/stringer/stringer.go new file mode 100644 index 0000000000..7ff0ee8d0c --- /dev/null +++ b/vendor/golang.org/x/tools/cmd/stringer/stringer.go @@ -0,0 +1,715 @@ +// Copyright 2014 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Stringer is a tool to automate the creation of methods that satisfy the fmt.Stringer +// interface. Given the name of a (signed or unsigned) integer type T that has constants +// defined, stringer will create a new self-contained Go source file implementing +// +// func (t T) String() string +// +// The file is created in the same package and directory as the package that defines T. +// It has helpful defaults designed for use with go generate. +// +// Stringer works best with constants that are consecutive values such as created using iota, +// but creates good code regardless. In the future it might also provide custom support for +// constant sets that are bit patterns. +// +// For example, given this snippet, +// +// package painkiller +// +// type Pill int +// +// const ( +// Placebo Pill = iota +// Aspirin +// Ibuprofen +// Paracetamol +// Acetaminophen = Paracetamol +// ) +// +// running this command +// +// stringer -type=Pill +// +// in the same directory will create the file pill_string.go, in package painkiller, +// containing a definition of +// +// func (Pill) String() string +// +// That method will translate the value of a Pill constant to the string representation +// of the respective constant name, so that the call fmt.Print(painkiller.Aspirin) will +// print the string "Aspirin". +// +// Typically this process would be run using go generate, like this: +// +// //go:generate stringer -type=Pill +// +// If multiple constants have the same value, the lexically first matching name will +// be used (in the example, Acetaminophen will print as "Paracetamol"). +// +// With no arguments, it processes the package in the current directory. +// Otherwise, the arguments must name a single directory holding a Go package +// or a set of Go source files that represent a single Go package. +// +// The -type flag accepts a comma-separated list of types so a single run can +// generate methods for multiple types. The default output file is t_string.go, +// where t is the lower-cased name of the first type listed. It can be overridden +// with the -output flag. +// +// Types can also be declared in tests, in which case type declarations in the +// non-test package or its test variant are preferred over types defined in the +// package with suffix "_test". +// The default output file for type declarations in tests is t_string_test.go with t picked as above. +// +// The -linecomment flag tells stringer to generate the text of any line comment, trimmed +// of leading spaces, instead of the constant name. For instance, if the constants above had a +// Pill prefix, one could write +// +// PillAspirin // Aspirin +// +// to suppress it in the output. +// +// The -trimprefix flag specifies a prefix to remove from the constant names +// when generating the string representations. For instance, -trimprefix=Pill +// would be an alternative way to ensure that PillAspirin.String() == "Aspirin". +package main // import "golang.org/x/tools/cmd/stringer" + +import ( + "bytes" + "flag" + "fmt" + "go/ast" + "go/constant" + "go/format" + "go/token" + "go/types" + "log" + "os" + "path/filepath" + "sort" + "strings" + + "golang.org/x/tools/go/packages" +) + +var ( + typeNames = flag.String("type", "", "comma-separated list of type names; must be set") + output = flag.String("output", "", "output file name; default srcdir/_string.go") + trimprefix = flag.String("trimprefix", "", "trim the `prefix` from the generated constant names") + linecomment = flag.Bool("linecomment", false, "use line comment text as printed text when present") + buildTags = flag.String("tags", "", "comma-separated list of build tags to apply") +) + +// Usage is a replacement usage function for the flags package. +func Usage() { + fmt.Fprintf(os.Stderr, "Usage of stringer:\n") + fmt.Fprintf(os.Stderr, "\tstringer [flags] -type T [directory]\n") + fmt.Fprintf(os.Stderr, "\tstringer [flags] -type T files... # Must be a single package\n") + fmt.Fprintf(os.Stderr, "For more information, see:\n") + fmt.Fprintf(os.Stderr, "\thttps://pkg.go.dev/golang.org/x/tools/cmd/stringer\n") + fmt.Fprintf(os.Stderr, "Flags:\n") + flag.PrintDefaults() +} + +func main() { + log.SetFlags(0) + log.SetPrefix("stringer: ") + flag.Usage = Usage + flag.Parse() + if len(*typeNames) == 0 { + flag.Usage() + os.Exit(2) + } + types := strings.Split(*typeNames, ",") + var tags []string + if len(*buildTags) > 0 { + tags = strings.Split(*buildTags, ",") + } + + // We accept either one directory or a list of files. Which do we have? + args := flag.Args() + if len(args) == 0 { + // Default: process whole package in current directory. + args = []string{"."} + } + + // Parse the package once. + var dir string + // TODO(suzmue): accept other patterns for packages (directories, list of files, import paths, etc). + if len(args) == 1 && isDirectory(args[0]) { + dir = args[0] + } else { + if len(tags) != 0 { + log.Fatal("-tags option applies only to directories, not when files are specified") + } + dir = filepath.Dir(args[0]) + } + + // For each type, generate code in the first package where the type is declared. + // The order of packages is as follows: + // package x + // package x compiled for tests + // package x_test + // + // Each package pass could result in a separate generated file. + // These files must have the same package and test/not-test nature as the types + // from which they were generated. + // + // Types will be excluded when generated, to avoid repetitions. + pkgs := loadPackages(args, tags, *trimprefix, *linecomment, nil /* logf */) + sort.Slice(pkgs, func(i, j int) bool { + // Put x_test packages last. + iTest := strings.HasSuffix(pkgs[i].name, "_test") + jTest := strings.HasSuffix(pkgs[j].name, "_test") + if iTest != jTest { + return !iTest + } + + return len(pkgs[i].files) < len(pkgs[j].files) + }) + for _, pkg := range pkgs { + g := Generator{ + pkg: pkg, + } + + // Print the header and package clause. + g.Printf("// Code generated by \"stringer %s\"; DO NOT EDIT.\n", strings.Join(os.Args[1:], " ")) + g.Printf("\n") + g.Printf("package %s", g.pkg.name) + g.Printf("\n") + g.Printf("import \"strconv\"\n") // Used by all methods. + + // Run generate for types that can be found. Keep the rest for the remainingTypes iteration. + var foundTypes, remainingTypes []string + for _, typeName := range types { + values := findValues(typeName, pkg) + if len(values) > 0 { + g.generate(typeName, values) + foundTypes = append(foundTypes, typeName) + } else { + remainingTypes = append(remainingTypes, typeName) + } + } + if len(foundTypes) == 0 { + // This package didn't have any of the relevant types, skip writing a file. + continue + } + if len(remainingTypes) > 0 && output != nil && *output != "" { + log.Fatalf("cannot write to single file (-output=%q) when matching types are found in multiple packages", *output) + } + types = remainingTypes + + // Format the output. + src := g.format() + + // Write to file. + outputName := *output + if outputName == "" { + // Type names will be unique across packages since only the first + // match is picked. + // So there won't be collisions between a package compiled for tests + // and the separate package of tests (package foo_test). + outputName = filepath.Join(dir, baseName(pkg, foundTypes[0])) + } + err := os.WriteFile(outputName, src, 0o644) + if err != nil { + log.Fatalf("writing output: %s", err) + } + } + + if len(types) > 0 { + log.Fatalf("no values defined for types: %s", strings.Join(types, ",")) + } +} + +// baseName that will put the generated code together with pkg. +func baseName(pkg *Package, typename string) string { + suffix := "string.go" + if pkg.hasTestFiles { + suffix = "string_test.go" + } + return fmt.Sprintf("%s_%s", strings.ToLower(typename), suffix) +} + +// isDirectory reports whether the named file is a directory. +func isDirectory(name string) bool { + info, err := os.Stat(name) + if err != nil { + log.Fatal(err) + } + return info.IsDir() +} + +// Generator holds the state of the analysis. Primarily used to buffer +// the output for format.Source. +type Generator struct { + buf bytes.Buffer // Accumulated output. + pkg *Package // Package we are scanning. + + logf func(format string, args ...any) // test logging hook; nil when not testing +} + +func (g *Generator) Printf(format string, args ...any) { + fmt.Fprintf(&g.buf, format, args...) +} + +// File holds a single parsed file and associated data. +type File struct { + pkg *Package // Package to which this file belongs. + file *ast.File // Parsed AST. + // These fields are reset for each type being generated. + typeName string // Name of the constant type. + values []Value // Accumulator for constant values of that type. + + trimPrefix string + lineComment bool +} + +type Package struct { + name string + defs map[*ast.Ident]types.Object + files []*File + hasTestFiles bool +} + +// loadPackages analyzes the single package constructed from the patterns and tags. +// loadPackages exits if there is an error. +// +// Returns all variants (such as tests) of the package. +// +// logf is a test logging hook. It can be nil when not testing. +func loadPackages( + patterns, tags []string, + trimPrefix string, lineComment bool, + logf func(format string, args ...any), +) []*Package { + cfg := &packages.Config{ + Mode: packages.NeedName | packages.NeedTypes | packages.NeedTypesInfo | packages.NeedSyntax | packages.NeedFiles, + // Tests are included, let the caller decide how to fold them in. + Tests: true, + BuildFlags: []string{fmt.Sprintf("-tags=%s", strings.Join(tags, " "))}, + Logf: logf, + } + pkgs, err := packages.Load(cfg, patterns...) + if err != nil { + log.Fatal(err) + } + if len(pkgs) == 0 { + log.Fatalf("error: no packages matching %v", strings.Join(patterns, " ")) + } + + out := make([]*Package, len(pkgs)) + for i, pkg := range pkgs { + p := &Package{ + name: pkg.Name, + defs: pkg.TypesInfo.Defs, + files: make([]*File, len(pkg.Syntax)), + } + + for j, file := range pkg.Syntax { + p.files[j] = &File{ + file: file, + pkg: p, + + trimPrefix: trimPrefix, + lineComment: lineComment, + } + } + + // Keep track of test files, since we might want to generated + // code that ends up in that kind of package. + // Can be replaced once https://go.dev/issue/38445 lands. + for _, f := range pkg.GoFiles { + if strings.HasSuffix(f, "_test.go") { + p.hasTestFiles = true + break + } + } + + out[i] = p + } + return out +} + +func findValues(typeName string, pkg *Package) []Value { + values := make([]Value, 0, 100) + for _, file := range pkg.files { + // Set the state for this run of the walker. + file.typeName = typeName + file.values = nil + if file.file != nil { + ast.Inspect(file.file, file.genDecl) + values = append(values, file.values...) + } + } + return values +} + +// generate produces the String method for the named type. +func (g *Generator) generate(typeName string, values []Value) { + // Generate code that will fail if the constants change value. + g.Printf("func _() {\n") + g.Printf("\t// An \"invalid array index\" compiler error signifies that the constant values have changed.\n") + g.Printf("\t// Re-run the stringer command to generate them again.\n") + g.Printf("\tvar x [1]struct{}\n") + for _, v := range values { + g.Printf("\t_ = x[%s - %s]\n", v.originalName, v.str) + } + g.Printf("}\n") + runs := splitIntoRuns(values) + // The decision of which pattern to use depends on the number of + // runs in the numbers. If there's only one, it's easy. For more than + // one, there's a tradeoff between complexity and size of the data + // and code vs. the simplicity of a map. A map takes more space, + // but so does the code. The decision here (crossover at 10) is + // arbitrary, but considers that for large numbers of runs the cost + // of the linear scan in the switch might become important, and + // rather than use yet another algorithm such as binary search, + // we punt and use a map. In any case, the likelihood of a map + // being necessary for any realistic example other than bitmasks + // is very low. And bitmasks probably deserve their own analysis, + // to be done some other day. + switch { + case len(runs) == 1: + g.buildOneRun(runs, typeName) + case len(runs) <= 10: + g.buildMultipleRuns(runs, typeName) + default: + g.buildMap(runs, typeName) + } +} + +// splitIntoRuns breaks the values into runs of contiguous sequences. +// For example, given 1,2,3,5,6,7 it returns {1,2,3},{5,6,7}. +// The input slice is known to be non-empty. +func splitIntoRuns(values []Value) [][]Value { + // We use stable sort so the lexically first name is chosen for equal elements. + sort.Stable(byValue(values)) + // Remove duplicates. Stable sort has put the one we want to print first, + // so use that one. The String method won't care about which named constant + // was the argument, so the first name for the given value is the only one to keep. + // We need to do this because identical values would cause the switch or map + // to fail to compile. + j := 1 + for i := 1; i < len(values); i++ { + if values[i].value != values[i-1].value { + values[j] = values[i] + j++ + } + } + values = values[:j] + runs := make([][]Value, 0, 10) + for len(values) > 0 { + // One contiguous sequence per outer loop. + i := 1 + for i < len(values) && values[i].value == values[i-1].value+1 { + i++ + } + runs = append(runs, values[:i]) + values = values[i:] + } + return runs +} + +// format returns the gofmt-ed contents of the Generator's buffer. +func (g *Generator) format() []byte { + src, err := format.Source(g.buf.Bytes()) + if err != nil { + // Should never happen, but can arise when developing this code. + // The user can compile the output to see the error. + log.Printf("warning: internal error: invalid Go generated: %s", err) + log.Printf("warning: compile the package to analyze the error") + return g.buf.Bytes() + } + return src +} + +// Value represents a declared constant. +type Value struct { + originalName string // The name of the constant. + name string // The name with trimmed prefix. + // The value is stored as a bit pattern alone. The boolean tells us + // whether to interpret it as an int64 or a uint64; the only place + // this matters is when sorting. + // Much of the time the str field is all we need; it is printed + // by Value.String. + value uint64 // Will be converted to int64 when needed. + signed bool // Whether the constant is a signed type. + str string // The string representation given by the "go/constant" package. +} + +func (v *Value) String() string { + return v.str +} + +// byValue lets us sort the constants into increasing order. +// We take care in the Less method to sort in signed or unsigned order, +// as appropriate. +type byValue []Value + +func (b byValue) Len() int { return len(b) } +func (b byValue) Swap(i, j int) { b[i], b[j] = b[j], b[i] } +func (b byValue) Less(i, j int) bool { + if b[i].signed { + return int64(b[i].value) < int64(b[j].value) + } + return b[i].value < b[j].value +} + +// genDecl processes one declaration clause. +func (f *File) genDecl(node ast.Node) bool { + decl, ok := node.(*ast.GenDecl) + if !ok || decl.Tok != token.CONST { + // We only care about const declarations. + return true + } + // The name of the type of the constants we are declaring. + // Can change if this is a multi-element declaration. + typ := "" + // Loop over the elements of the declaration. Each element is a ValueSpec: + // a list of names possibly followed by a type, possibly followed by values. + // If the type and value are both missing, we carry down the type (and value, + // but the "go/types" package takes care of that). + for _, spec := range decl.Specs { + vspec := spec.(*ast.ValueSpec) // Guaranteed to succeed as this is CONST. + if vspec.Type == nil && len(vspec.Values) > 0 { + // "X = 1". With no type but a value. If the constant is untyped, + // skip this vspec and reset the remembered type. + typ = "" + + // If this is a simple type conversion, remember the type. + // We don't mind if this is actually a call; a qualified call won't + // be matched (that will be SelectorExpr, not Ident), and only unusual + // situations will result in a function call that appears to be + // a type conversion. + ce, ok := vspec.Values[0].(*ast.CallExpr) + if !ok { + continue + } + id, ok := ce.Fun.(*ast.Ident) + if !ok { + continue + } + typ = id.Name + } + if vspec.Type != nil { + // "X T". We have a type. Remember it. + ident, ok := vspec.Type.(*ast.Ident) + if !ok { + continue + } + typ = ident.Name + } + if typ != f.typeName { + // This is not the type we're looking for. + continue + } + // We now have a list of names (from one line of source code) all being + // declared with the desired type. + // Grab their names and actual values and store them in f.values. + for _, name := range vspec.Names { + if name.Name == "_" { + continue + } + // This dance lets the type checker find the values for us. It's a + // bit tricky: look up the object declared by the name, find its + // types.Const, and extract its value. + obj, ok := f.pkg.defs[name] + if !ok { + log.Fatalf("no value for constant %s", name) + } + info := obj.Type().Underlying().(*types.Basic).Info() + if info&types.IsInteger == 0 { + log.Fatalf("can't handle non-integer constant type %s", typ) + } + value := obj.(*types.Const).Val() // Guaranteed to succeed as this is CONST. + if value.Kind() != constant.Int { + log.Fatalf("can't happen: constant is not an integer %s", name) + } + i64, isInt := constant.Int64Val(value) + u64, isUint := constant.Uint64Val(value) + if !isInt && !isUint { + log.Fatalf("internal error: value of %s is not an integer: %s", name, value.String()) + } + if !isInt { + u64 = uint64(i64) + } + v := Value{ + originalName: name.Name, + value: u64, + signed: info&types.IsUnsigned == 0, + str: value.String(), + } + if c := vspec.Comment; f.lineComment && c != nil && len(c.List) == 1 { + v.name = strings.TrimSpace(c.Text()) + } else { + v.name = strings.TrimPrefix(v.originalName, f.trimPrefix) + } + f.values = append(f.values, v) + } + } + return false +} + +// Helpers + +// usize returns the number of bits of the smallest unsigned integer +// type that will hold n. Used to create the smallest possible slice of +// integers to use as indexes into the concatenated strings. +func usize(n int) int { + switch { + case n < 1<<8: + return 8 + case n < 1<<16: + return 16 + default: + // 2^32 is enough constants for anyone. + return 32 + } +} + +// declareIndexAndNameVars declares the index slices and concatenated names +// strings representing the runs of values. +func (g *Generator) declareIndexAndNameVars(runs [][]Value, typeName string) { + var indexes, names []string + for i, run := range runs { + index, name := g.createIndexAndNameDecl(run, typeName, fmt.Sprintf("_%d", i)) + if len(run) != 1 { + indexes = append(indexes, index) + } + names = append(names, name) + } + g.Printf("const (\n") + for _, name := range names { + g.Printf("\t%s\n", name) + } + g.Printf(")\n\n") + + if len(indexes) > 0 { + g.Printf("var (") + for _, index := range indexes { + g.Printf("\t%s\n", index) + } + g.Printf(")\n\n") + } +} + +// declareIndexAndNameVar is the single-run version of declareIndexAndNameVars +func (g *Generator) declareIndexAndNameVar(run []Value, typeName string) { + index, name := g.createIndexAndNameDecl(run, typeName, "") + g.Printf("const %s\n", name) + g.Printf("var %s\n", index) +} + +// createIndexAndNameDecl returns the pair of declarations for the run. The caller will add "const" and "var". +func (g *Generator) createIndexAndNameDecl(run []Value, typeName string, suffix string) (string, string) { + b := new(bytes.Buffer) + indexes := make([]int, len(run)) + for i := range run { + b.WriteString(run[i].name) + indexes[i] = b.Len() + } + nameConst := fmt.Sprintf("_%s_name%s = %q", typeName, suffix, b.String()) + nameLen := b.Len() + b.Reset() + fmt.Fprintf(b, "_%s_index%s = [...]uint%d{0, ", typeName, suffix, usize(nameLen)) + for i, v := range indexes { + if i > 0 { + fmt.Fprintf(b, ", ") + } + fmt.Fprintf(b, "%d", v) + } + fmt.Fprintf(b, "}") + return b.String(), nameConst +} + +// declareNameVars declares the concatenated names string representing all the values in the runs. +func (g *Generator) declareNameVars(runs [][]Value, typeName string, suffix string) { + g.Printf("const _%s_name%s = \"", typeName, suffix) + for _, run := range runs { + for i := range run { + g.Printf("%s", run[i].name) + } + } + g.Printf("\"\n") +} + +// buildOneRun generates the variables and String method for a single run of contiguous values. +func (g *Generator) buildOneRun(runs [][]Value, typeName string) { + values := runs[0] + g.Printf("\n") + g.declareIndexAndNameVar(values, typeName) + g.Printf(stringOneRun, typeName, values[0].String()) +} + +// Arguments to format are: +// +// [1]: type name +// [2]: lowest defined value for type, as a string +const stringOneRun = `func (i %[1]s) String() string { + idx := int(i) - %[2]s + if i < %[2]s || idx >= len(_%[1]s_index)-1 { + return "%[1]s(" + strconv.FormatInt(int64(i), 10) + ")" + } + return _%[1]s_name[_%[1]s_index[idx] : _%[1]s_index[idx+1]] +} +` + +// buildMultipleRuns generates the variables and String method for multiple runs of contiguous values. +// For this pattern, a single Printf format won't do. +func (g *Generator) buildMultipleRuns(runs [][]Value, typeName string) { + g.Printf("\n") + g.declareIndexAndNameVars(runs, typeName) + g.Printf("func (i %s) String() string {\n", typeName) + g.Printf("\tswitch {\n") + for i, values := range runs { + if len(values) == 1 { + g.Printf("\tcase i == %s:\n", &values[0]) + g.Printf("\t\treturn _%s_name_%d\n", typeName, i) + continue + } + if values[0].value == 0 && !values[0].signed { + // For an unsigned lower bound of 0, "0 <= i" would be redundant. + g.Printf("\tcase i <= %s:\n", &values[len(values)-1]) + } else { + g.Printf("\tcase %s <= i && i <= %s:\n", &values[0], &values[len(values)-1]) + } + if values[0].value != 0 { + g.Printf("\t\ti -= %s\n", &values[0]) + } + g.Printf("\t\treturn _%s_name_%d[_%s_index_%d[i]:_%s_index_%d[i+1]]\n", + typeName, i, typeName, i, typeName, i) + } + g.Printf("\tdefault:\n") + g.Printf("\t\treturn \"%s(\" + strconv.FormatInt(int64(i), 10) + \")\"\n", typeName) + g.Printf("\t}\n") + g.Printf("}\n") +} + +// buildMap handles the case where the space is so sparse a map is a reasonable fallback. +// It's a rare situation but has simple code. +func (g *Generator) buildMap(runs [][]Value, typeName string) { + g.Printf("\n") + g.declareNameVars(runs, typeName, "") + g.Printf("\nvar _%s_map = map[%s]string{\n", typeName, typeName) + n := 0 + for _, values := range runs { + for _, value := range values { + g.Printf("\t%s: _%s_name[%d:%d],\n", &value, typeName, n, n+len(value.name)) + n += len(value.name) + } + } + g.Printf("}\n\n") + g.Printf(stringMap, typeName) +} + +// Argument to format is the type name. +const stringMap = `func (i %[1]s) String() string { + if str, ok := _%[1]s_map[i]; ok { + return str + } + return "%[1]s(" + strconv.FormatInt(int64(i), 10) + ")" +} +` diff --git a/vendor/modules.txt b/vendor/modules.txt index 735fc4d928..e7fe96bf7c 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1784,6 +1784,7 @@ golang.org/x/text/width golang.org/x/time/rate # golang.org/x/tools v0.42.0 ## explicit; go 1.24.0 +golang.org/x/tools/cmd/stringer golang.org/x/tools/cover golang.org/x/tools/go/analysis golang.org/x/tools/go/analysis/passes/appends