diff --git a/Dockerfile b/Dockerfile index 085a64e85..156f6992a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -9,6 +9,7 @@ FROM registry.ci.openshift.org/ocp/5.0:base-rhel9 COPY --from=builder /go/src/github.com/openshift/cluster-cloud-controller-manager-operator/bin/cluster-controller-manager-operator . COPY --from=builder /go/src/github.com/openshift/cluster-cloud-controller-manager-operator/bin/config-sync-controllers . COPY --from=builder /go/src/github.com/openshift/cluster-cloud-controller-manager-operator/bin/azure-config-credentials-injector . +COPY --from=builder /go/src/github.com/openshift/cluster-cloud-controller-manager-operator/bin/vsphere-node-label-sync-job . COPY --from=builder /go/src/github.com/openshift/cluster-cloud-controller-manager-operator/manifests manifests COPY --from=builder /go/src/github.com/openshift/cluster-cloud-controller-manager-operator/openshift-tests/bin/cloud-controller-manager-aws-tests-ext.gz /usr/bin/cloud-controller-manager-aws-tests-ext.gz COPY --from=builder /go/src/github.com/openshift/cluster-cloud-controller-manager-operator/openshift-tests/bin/cloud-controller-manager-operator-tests-ext.gz /usr/bin/cloud-controller-manager-operator-tests-ext.gz diff --git a/Makefile b/Makefile index 8d995234e..6b6efc9f8 100644 --- a/Makefile +++ b/Makefile @@ -39,7 +39,7 @@ unit: # Build operator binaries .PHONY: build -build: operator config-sync-controllers azure-config-credentials-injector cloud-controller-manager-aws-tests-ext cluster-cloud-controller-manager-operator-tests-ext +build: operator config-sync-controllers azure-config-credentials-injector vsphere-node-label-sync-job cloud-controller-manager-aws-tests-ext cluster-cloud-controller-manager-operator-tests-ext operator: go build -o bin/cluster-controller-manager-operator cmd/cluster-cloud-controller-manager-operator/main.go @@ -50,6 +50,9 @@ config-sync-controllers: azure-config-credentials-injector: go build -o bin/azure-config-credentials-injector cmd/azure-config-credentials-injector/main.go +vsphere-node-label-sync-job: + go build -o bin/vsphere-node-label-sync-job cmd/vsphere-node-label-sync-job/main.go + cloud-controller-manager-aws-tests-ext: cd openshift-tests/ccm-aws-tests && \ mkdir -p ../bin && \ diff --git a/cmd/vsphere-node-label-sync-job/main.go b/cmd/vsphere-node-label-sync-job/main.go new file mode 100644 index 000000000..6e82c4530 --- /dev/null +++ b/cmd/vsphere-node-label-sync-job/main.go @@ -0,0 +1,134 @@ +/* +Copyright 2021. + +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. +*/ + +// Command vsphere-node-label-sync-job runs to completion once, backfilling the +// node.openshift.io/platform-type=vsphere label onto vSphere nodes that are missing it. It is +// installed as a Kubernetes Job by the CVO (see +// manifests/0000_26_cloud-controller-manager-operator_45_job-vsphere-node-label-sync.yaml) rather +// than run as an ongoing in-process controller. +package main + +import ( + "context" + "errors" + "flag" + "os" + "time" + + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + + // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) + // to ensure that exec-entrypoint and run can make use of them. + "k8s.io/client-go/kubernetes" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + _ "k8s.io/client-go/plugin/pkg/client/auth" + "k8s.io/klog/v2" + "k8s.io/utils/clock" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + configv1 "github.com/openshift/api/config/v1" + configv1client "github.com/openshift/client-go/config/clientset/versioned" + configinformers "github.com/openshift/client-go/config/informers/externalversions" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" + "github.com/openshift/library-go/pkg/operator/events" + + "github.com/openshift/cluster-cloud-controller-manager-operator/pkg/controllers" +) + +var ( + scheme = runtime.NewScheme() + setupLog = ctrl.Log.WithName("setup") +) + +const ( + recorderName = "cloud-controller-manager-operator-vsphere-node-label-sync-job" + missingVersion = "0.0.1-snapshot" + managedNamespace = controllers.DefaultManagedNamespace + featureGateWaitTimeout = 1 * time.Minute + // jobTimeout bounds the process's overall runtime safely below the Job manifest's + // activeDeadlineSeconds (300s), so it can log a clear error and exit cleanly instead of + // being killed by the kubelet once the deadline is hit. + jobTimeout = 4 * time.Minute +) + +func init() { + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(configv1.AddToScheme(scheme)) +} + +func main() { + klog.InitFlags(flag.CommandLine) + flag.Parse() + + ctrl.SetLogger(klog.NewKlogr().WithName("VSphereNodeLabelSyncJob")) + + ctx, cancel := context.WithTimeout(ctrl.SetupSignalHandler(), jobTimeout) + defer cancel() + + restConfig := ctrl.GetConfigOrDie() + + k8sClient, err := client.New(restConfig, client.Options{Scheme: scheme}) + if err != nil { + setupLog.Error(err, "unable to create client") + os.Exit(1) + } + + configClient, err := configv1client.NewForConfig(restConfig) + if err != nil { + setupLog.Error(err, "unable to create config client") + os.Exit(1) + } + + kubeClient, err := kubernetes.NewForConfig(restConfig) + if err != nil { + setupLog.Error(err, "unable to create kube client") + os.Exit(1) + } + + configInformers := configinformers.NewSharedInformerFactory(configClient, 10*time.Minute) + controllerRef, err := events.GetControllerReferenceForCurrentPod(ctx, kubeClient, managedNamespace, nil) + if err != nil { + klog.Warningf("unable to get owner reference (falling back to namespace): %v", err) + } + + featureGateAccessor := featuregates.NewFeatureGateAccess( + controllers.GetReleaseVersion(), missingVersion, + configInformers.Config().V1().ClusterVersions(), configInformers.Config().V1().FeatureGates(), + events.NewKubeRecorder(kubeClient.CoreV1().Events(managedNamespace), recorderName, controllerRef, clock.RealClock{}), + ) + featureGateAccessor.SetChangeHandler(func(featuregates.FeatureChange) {}) + go featureGateAccessor.Run(ctx) + go configInformers.Start(ctx.Done()) + + select { + case <-featureGateAccessor.InitialFeatureGatesObserved(): + case <-ctx.Done(): + setupLog.Error(ctx.Err(), "context canceled while waiting for FeatureGate detection") + os.Exit(1) + case <-time.After(featureGateWaitTimeout): + setupLog.Error(errors.New("timed out waiting for FeatureGate detection"), "unable to determine feature gate state") + os.Exit(1) + } + + if err := controllers.SyncVSphereNodeLabels(ctx, k8sClient, featureGateAccessor); err != nil { + setupLog.Error(err, "failed to sync vSphere node labels") + os.Exit(1) + } + + setupLog.Info("vSphere node label sync completed successfully") +} diff --git a/manifests/0000_26_cloud-controller-manager-operator_45_job-vsphere-node-label-sync.yaml b/manifests/0000_26_cloud-controller-manager-operator_45_job-vsphere-node-label-sync.yaml new file mode 100644 index 000000000..f78363489 --- /dev/null +++ b/manifests/0000_26_cloud-controller-manager-operator_45_job-vsphere-node-label-sync.yaml @@ -0,0 +1,118 @@ +apiVersion: batch/v1 +kind: Job +metadata: + name: vsphere-node-label-sync + namespace: openshift-cloud-controller-manager-operator + annotations: + capability.openshift.io/name: CloudControllerManager + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + # Only create this Job on clusters where the VSphereMixedNodeEnv feature gate is enabled, + # matching the same gate the vSphere CCM checks before applying the node.openshift.io/platform-type + # label at node-init time (see pkg/cloud/vsphere/vsphere.go). The CVO re-evaluates this against the + # live FeatureGate object on every sync, so the Job is created as soon as the gate turns on. + release.openshift.io/feature-gate: "VSphereMixedNodeEnv" + # This Job must only ever be created once, never updated. Its pod template's container image + # changes on nearly every release, but a Job's pod template is immutable once created, so if + # the CVO ever tried to reconcile this Job in place (instead of only creating it) the update + # would fail on every subsequent upgrade after the first. create-only makes the CVO create it + # once and leave it alone from then on, matching the intended "backfill once" semantics. + release.openshift.io/create-only: "true" + labels: + k8s-app: vsphere-node-label-sync +spec: + backoffLimit: 6 + activeDeadlineSeconds: 300 + template: + metadata: + labels: + k8s-app: vsphere-node-label-sync + annotations: + target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' + openshift.io/required-scc: hostaccess + spec: + priorityClassName: system-node-critical + serviceAccountName: cluster-cloud-controller-manager + restartPolicy: OnFailure + # This Job may run before pod networking (CNI/OVN service routing) is functional on a given + # node -- e.g. right after a control-plane node reboots during an upgrade. hostNetwork plus + # sourcing /etc/kubernetes/apiserver-url.env (mirroring the operator Deployment in + # 0000_26_cloud-controller-manager-operator_50_deployment.yaml) lets the container reach the + # API server directly instead of through the "kubernetes" Service ClusterIP, which requires + # kube-proxy/OVN to already be wired up. That file is only populated on control-plane nodes, + # hence the master nodeSelector/tolerations. + hostNetwork: true + nodeSelector: + node-role.kubernetes.io/master: "" + tolerations: + - key: node.cloudprovider.kubernetes.io/uninitialized + effect: NoSchedule + value: "true" + - key: "node-role.kubernetes.io/master" + operator: "Exists" + effect: "NoSchedule" + - key: "node.kubernetes.io/unreachable" + operator: "Exists" + effect: "NoExecute" + tolerationSeconds: 120 + - key: "node.kubernetes.io/not-ready" + operator: "Exists" + effect: "NoExecute" + tolerationSeconds: 120 + - key: "node.cloudprovider.kubernetes.io/uninitialized" + operator: "Exists" + effect: "NoSchedule" + # CNI relies on CCM to fill in IP information on Node objects. + # Therefore we must schedule before the CNI can mark the Node as ready. + - key: "node.kubernetes.io/not-ready" + operator: "Exists" + effect: "NoSchedule" + containers: + - name: vsphere-node-label-sync + image: quay.io/openshift/origin-cluster-cloud-controller-manager-operator + command: + - /bin/bash + - -c + - | + #!/bin/bash + set -o allexport + if [[ -f /etc/kubernetes/apiserver-url.env ]]; then + source /etc/kubernetes/apiserver-url.env + else + URL_ONLY_KUBECONFIG=/etc/kubernetes/kubeconfig + fi + exec /vsphere-node-label-sync-job + env: + - name: RELEASE_VERSION + value: "0.0.1-snapshot" + resources: + requests: + cpu: 10m + memory: 50Mi + limits: + cpu: 100m + memory: 100Mi + terminationMessagePolicy: FallbackToLogsOnError + # runAsNonRoot is deliberately NOT set here: this image (like the operator's own + # Deployment container, which also runs unguarded) has no USER directive in its + # Dockerfile and defaults to root, and the hostaccess SCC does not reliably assign + # a non-root UID for this pod. Setting runAsNonRoot: true without a matching + # runAsUser makes the kubelet refuse to start the container at all ("container has + # runAsNonRoot and image will run as root"), which is what happened in CI (PR #497, + # e2e-azure-ovn-upgrade): the container crash-looped for the full activeDeadlineSeconds + # window and tripped the "events should not repeat pathologically" invariant test. + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + capabilities: + drop: + - ALL + volumeMounts: + - mountPath: /etc/kubernetes + name: host-etc-kube + readOnly: true + volumes: + - name: host-etc-kube + hostPath: + path: /etc/kubernetes + type: Directory diff --git a/pkg/cloud/vsphere/vsphere.go b/pkg/cloud/vsphere/vsphere.go index 4801c70d1..80f9e6797 100644 --- a/pkg/cloud/vsphere/vsphere.go +++ b/pkg/cloud/vsphere/vsphere.go @@ -20,7 +20,16 @@ const ( // see manifests/0000_26_cloud-controller-manager-operator_16_credentialsrequest-vsphere.yaml globalCredsSecretName = "vsphere-cloud-credentials" - vSpherePlatformTypeLabel = "node.openshift.io/platform-type=vsphere" + // NodePlatformTypeLabelKey and NodePlatformTypeLabelValueVSphere are the label the vSphere CCM applies + // (via --node-labels, see additionalLabels below) to nodes when it first initializes them. They are + // exported so that node.openshift.io/platform-type=vsphere can be reapplied to nodes that were + // initialized before the VSphereMixedNodeEnv feature gate was enabled. + NodePlatformTypeLabelKey = "node.openshift.io/platform-type" + NodePlatformTypeLabelValueVSphere = "vsphere" + // NodeProviderIDPrefix is the spec.providerID prefix used by vSphere nodes, e.g. vsphere://4210e24f-... + NodeProviderIDPrefix = "vsphere://" + + vSpherePlatformTypeLabel = NodePlatformTypeLabelKey + "=" + NodePlatformTypeLabelValueVSphere ) var ( diff --git a/pkg/controllers/vsphere_node_label_sync.go b/pkg/controllers/vsphere_node_label_sync.go new file mode 100644 index 000000000..dd5c6c18f --- /dev/null +++ b/pkg/controllers/vsphere_node_label_sync.go @@ -0,0 +1,91 @@ +package controllers + +import ( + "context" + "errors" + "fmt" + "strings" + + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/api/features" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/vsphere" +) + +// SyncVSphereNodeLabels performs a single pass over all nodes and retroactively applies the +// node.openshift.io/platform-type=vsphere label to vSphere nodes that are missing it. The vSphere +// CCM only ever applies this label (via its --node-labels flag, see pkg/cloud/vsphere) when it +// initializes a node for the first time, so nodes that joined the cluster before the +// VSphereMixedNodeEnv feature gate was enabled never get labeled without this repair pass. Hybrid +// clusters can mix vSphere and bare-metal nodes, so each node's spec.providerID is checked to +// confirm it is actually a vSphere node before labeling it. +// +// This is run to completion once by the vsphere-node-label-sync-job Job that the CVO installs +// (see manifests/0000_26_cloud-controller-manager-operator_45_job-vsphere-node-label-sync.yaml), +// rather than as an ongoing in-process controller. +func SyncVSphereNodeLabels(ctx context.Context, c client.Client, featureGateAccess featuregates.FeatureGateAccess) error { + infra := &configv1.Infrastructure{} + if err := c.Get(ctx, client.ObjectKey{Name: infrastructureResourceName}, infra); err != nil { + if apierrors.IsNotFound(err) { + klog.Infof("infrastructure resource not found, skipping node label sync") + return nil + } + return fmt.Errorf("failed to get infrastructure: %w", err) + } + + if infra.Status.PlatformStatus == nil || infra.Status.PlatformStatus.Type != configv1.VSpherePlatformType { + klog.V(2).Infof("platform is not vSphere, skipping node label sync") + return nil + } + + currentFeatureGates, err := featureGateAccess.CurrentFeatureGates() + if err != nil { + return fmt.Errorf("failed to get current feature gates: %w", err) + } + if !currentFeatureGates.Enabled(features.FeatureGateVSphereMixedNodeEnv) { + klog.V(2).Infof("%s feature gate is disabled, skipping node label sync", features.FeatureGateVSphereMixedNodeEnv) + return nil + } + + nodeList := &corev1.NodeList{} + if err := c.List(ctx, nodeList); err != nil { + return fmt.Errorf("failed to list nodes: %w", err) + } + + var errs []error + for i := range nodeList.Items { + if err := syncVSphereNodeLabel(ctx, c, &nodeList.Items[i]); err != nil { + errs = append(errs, fmt.Errorf("node %s: %w", nodeList.Items[i].Name, err)) + } + } + + return errors.Join(errs...) +} + +func syncVSphereNodeLabel(ctx context.Context, c client.Client, node *corev1.Node) error { + if !strings.HasPrefix(node.Spec.ProviderID, vsphere.NodeProviderIDPrefix) { + return nil + } + + if node.Labels[vsphere.NodePlatformTypeLabelKey] == vsphere.NodePlatformTypeLabelValueVSphere { + return nil + } + + patch := client.MergeFrom(node.DeepCopy()) + if node.Labels == nil { + node.Labels = map[string]string{} + } + node.Labels[vsphere.NodePlatformTypeLabelKey] = vsphere.NodePlatformTypeLabelValueVSphere + + if err := c.Patch(ctx, node, patch); err != nil { + return fmt.Errorf("failed to patch label: %w", err) + } + + klog.Infof("added %s=%s label to node %s", vsphere.NodePlatformTypeLabelKey, vsphere.NodePlatformTypeLabelValueVSphere, node.Name) + return nil +} diff --git a/pkg/controllers/vsphere_node_label_sync_test.go b/pkg/controllers/vsphere_node_label_sync_test.go new file mode 100644 index 000000000..649ed6c44 --- /dev/null +++ b/pkg/controllers/vsphere_node_label_sync_test.go @@ -0,0 +1,110 @@ +package controllers + +import ( + "context" + "testing" + + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/api/features" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/vsphere" +) + +func newTestInfrastructure(platformType configv1.PlatformType) *configv1.Infrastructure { + return &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{Name: infrastructureResourceName}, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{Type: platformType}, + }, + } +} + +func newTestNode(name, providerID string, labels map[string]string) *corev1.Node { + return &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: name, Labels: labels}, + Spec: corev1.NodeSpec{ProviderID: providerID}, + } +} + +func TestSyncVSphereNodeLabels(t *testing.T) { + vsphereNodeMissingLabel := newTestNode("vsphere-missing-label", "vsphere://4210e24f-8828-9387-8b6d-d4ebab373827", nil) + vsphereNodeAlreadyLabeled := newTestNode("vsphere-already-labeled", "vsphere://another-uuid", map[string]string{ + vsphere.NodePlatformTypeLabelKey: vsphere.NodePlatformTypeLabelValueVSphere, + }) + bareMetalNode := newTestNode("bare-metal", "baremetalhost://some-id", nil) + noProviderIDNode := newTestNode("no-provider-id", "", nil) + + testCases := []struct { + name string + platformType configv1.PlatformType + featureGateEnabled bool + nodes []*corev1.Node + expectLabeled []string + expectNotLabeled []string + }{ + { + name: "labels vsphere node missing the label when gate enabled on vSphere platform", + platformType: configv1.VSpherePlatformType, + featureGateEnabled: true, + nodes: []*corev1.Node{vsphereNodeMissingLabel, vsphereNodeAlreadyLabeled, bareMetalNode, noProviderIDNode}, + expectLabeled: []string{"vsphere-missing-label", "vsphere-already-labeled"}, + expectNotLabeled: []string{"bare-metal", "no-provider-id"}, + }, + { + name: "does nothing when feature gate is disabled", + platformType: configv1.VSpherePlatformType, + featureGateEnabled: false, + nodes: []*corev1.Node{vsphereNodeMissingLabel}, + expectNotLabeled: []string{"vsphere-missing-label"}, + }, + { + name: "does nothing on non-vSphere platforms", + platformType: configv1.AWSPlatformType, + featureGateEnabled: true, + nodes: []*corev1.Node{vsphereNodeMissingLabel}, + expectNotLabeled: []string{"vsphere-missing-label"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + objs := []client.Object{newTestInfrastructure(tc.platformType)} + for _, n := range tc.nodes { + objs = append(objs, n.DeepCopy()) + } + + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(objs...).Build() + + var featureGateAccess featuregates.FeatureGateAccess + if tc.featureGateEnabled { + featureGateAccess = featuregates.NewHardcodedFeatureGateAccess( + []configv1.FeatureGateName{features.FeatureGateVSphereMixedNodeEnv}, nil) + } else { + featureGateAccess = featuregates.NewHardcodedFeatureGateAccess(nil, + []configv1.FeatureGateName{features.FeatureGateVSphereMixedNodeEnv}) + } + + require.NoError(t, SyncVSphereNodeLabels(context.Background(), fakeClient, featureGateAccess)) + + for _, name := range tc.expectLabeled { + node := &corev1.Node{} + require.NoError(t, fakeClient.Get(context.Background(), client.ObjectKey{Name: name}, node)) + assert.Equal(t, vsphere.NodePlatformTypeLabelValueVSphere, node.Labels[vsphere.NodePlatformTypeLabelKey], "node %s should be labeled", name) + } + + for _, name := range tc.expectNotLabeled { + node := &corev1.Node{} + require.NoError(t, fakeClient.Get(context.Background(), client.ObjectKey{Name: name}, node)) + assert.NotEqual(t, vsphere.NodePlatformTypeLabelValueVSphere, node.Labels[vsphere.NodePlatformTypeLabelKey], "node %s should not be labeled", name) + } + }) + } +}