diff --git a/cmd/machine-config-controller/start.go b/cmd/machine-config-controller/start.go index 3991178653..73242398b2 100644 --- a/cmd/machine-config-controller/start.go +++ b/cmd/machine-config-controller/start.go @@ -202,15 +202,14 @@ func runStartCmd(_ *cobra.Command, _ []string) { go pinnedImageSet.Run(ctx, 2) // Start the shared factory informers that you need to use in your controller - ctrlctx.InformerFactory.Start(ctrlctx.Stop) - ctrlctx.KubeInformerFactory.Start(ctrlctx.Stop) - ctrlctx.OpenShiftConfigKubeNamespacedInformerFactory.Start(ctrlctx.Stop) - ctrlctx.OperatorInformerFactory.Start(ctrlctx.Stop) - ctrlctx.ConfigInformerFactory.Start(ctrlctx.Stop) - ctrlctx.KubeNamespacedInformerFactory.Start(ctrlctx.Stop) - ctrlctx.KubeMAOSharedInformer.Start(ctrlctx.Stop) - ctrlctx.OCLInformerFactory.Start(ctrlctx.Stop) - ctrlctx.MCOPodInformerFactory.Start(ctrlctx.Stop) + ctrlctx.InformerFactory.Start(ctx.Done()) + ctrlctx.KubeInformerFactory.Start(ctx.Done()) + ctrlctx.OpenShiftConfigKubeNamespacedInformerFactory.Start(ctx.Done()) + ctrlctx.OperatorInformerFactory.Start(ctx.Done()) + ctrlctx.ConfigInformerFactory.Start(ctx.Done()) + ctrlctx.KubeNamespacedInformerFactory.Start(ctx.Done()) + ctrlctx.KubeMAOSharedInformer.Start(ctx.Done()) + ctrlctx.OCLInformerFactory.Start(ctx.Done()) close(ctrlctx.InformersStarted) @@ -374,7 +373,7 @@ func createControllers(ctx *ctrlcommon.ControllerContext, inspectionCache *image ctx.InformerFactory.Machineconfiguration().V1().MachineConfigs(), ctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(), ctx.KubeInformerFactory.Core().V1().Nodes(), - ctx.MCOPodInformerFactory.Core().V1().Pods(), + ctx.KubeInformerFactory.Core().V1().Pods(), ctx.OCLInformerFactory.Machineconfiguration().V1().MachineOSConfigs(), ctx.OCLInformerFactory.Machineconfiguration().V1().MachineOSBuilds(), ctx.InformerFactory.Machineconfiguration().V1().MachineConfigNodes(), diff --git a/cmd/machine-config-daemon/start.go b/cmd/machine-config-daemon/start.go index 6e97a5c1e6..110fac82d4 100644 --- a/cmd/machine-config-daemon/start.go +++ b/cmd/machine-config-daemon/start.go @@ -236,15 +236,12 @@ func runStartCmd(_ *cobra.Command, _ []string) { ) go pinnedImageSetManager.Run(2, stopCh) - var mcnScopedInformerStartFunc func(<-chan struct{}) if ctrlctx.FeatureGatesHandler.Enabled(features.FeatureGateNoRegistryClusterInstall) { - mcnScopedInformer, startFunc := ctrlcommon.NewScopedMachineConfigNodeInformerFromClientBuilder(cb, startOpts.nodeName) - mcnScopedInformerStartFunc = startFunc internalReleaseImageManager := internalreleaseimage.New( startOpts.nodeName, ctrlctx.ClientBuilder.MachineConfigClientOrDie(componentName), ctrlctx.InformerFactory.Machineconfiguration().V1().InternalReleaseImages(), - mcnScopedInformer, + ctrlctx.InformerFactory.Machineconfiguration().V1().MachineConfigNodes(), ) go internalReleaseImageManager.Run(1, stopCh) } @@ -254,9 +251,6 @@ func runStartCmd(_ *cobra.Command, _ []string) { ctrlctx.InformerFactory.Start(stopCh) ctrlctx.OperatorInformerFactory.Start(stopCh) nodeScopedInformerStartFunc(ctrlctx.Stop) - if mcnScopedInformerStartFunc != nil { - mcnScopedInformerStartFunc(ctrlctx.Stop) - } close(ctrlctx.InformersStarted) if err := dn.Run(stopCh, exitCh, errCh); err != nil { diff --git a/cmd/machine-os-builder/start.go b/cmd/machine-os-builder/start.go index 1e1fc4dd1d..a1c20f795c 100644 --- a/cmd/machine-os-builder/start.go +++ b/cmd/machine-os-builder/start.go @@ -25,19 +25,13 @@ var ( } startOpts struct { - kubeconfig string - promMetricsListenAddress string - tlsCipherSuites []string - tlsMinVersion string + kubeconfig string } ) func init() { rootCmd.AddCommand(startCmd) startCmd.PersistentFlags().StringVar(&startOpts.kubeconfig, "kubeconfig", "", "Kubeconfig file to access a remote cluster (testing only)") - startCmd.PersistentFlags().StringVar(&startOpts.promMetricsListenAddress, "metrics-listen-address", "127.0.0.1:8797", "Listen address for prometheus metrics listener") - startCmd.PersistentFlags().StringSliceVar(&startOpts.tlsCipherSuites, "tls-cipher-suites", nil, "Comma-separated list of cipher suites for the metrics server") - startCmd.PersistentFlags().StringVar(&startOpts.tlsMinVersion, "tls-min-version", "VersionTLS12", "Minimum TLS version supported for the metrics server") } func runStartCmd(_ *cobra.Command, _ []string) { @@ -67,9 +61,6 @@ func runStartCmd(_ *cobra.Command, _ []string) { ctrlCtx := ctrlcommon.CreateControllerContext(ctx, cb) - // Start the metrics listener for OCL telemetry - go ctrlcommon.StartMetricsListener(startOpts.promMetricsListenAddress, ctrlCtx.Stop, build.RegisterOCLMetrics, startOpts.tlsMinVersion, startOpts.tlsCipherSuites) - ctrl := build.NewOSBuildControllerFromControllerContext(ctrlCtx) // Wire up our shutdown channel. diff --git a/install/0000_90_machine-config_01_prometheus-rules.yaml b/install/0000_90_machine-config_01_prometheus-rules.yaml index 27b2316900..16de9bb88a 100644 --- a/install/0000_90_machine-config_01_prometheus-rules.yaml +++ b/install/0000_90_machine-config_01_prometheus-rules.yaml @@ -15,10 +15,6 @@ spec: rules: - expr: sum(os_image_url_override) record: os_image_url_override:sum - - name: ocl-adoption.rules - rules: - - expr: mco_mosc_count - record: mco_mosc_count:sum - name: runc-deprecated rules: - alert: RuncDeprecated @@ -69,46 +65,7 @@ spec: severity: info annotations: summary: "Boot image skew enforcement is disabled. Scaling operations may not be successful." - description: "Boot image skew enforcement mode is set to None. When scaling up, new nodes may be provisioned with older boot images that could introduce compatibility issues. Consider manually updating boot images to match the cluster version. Please refer to docs at https://docs.redhat.com/en/documentation/openshift_container_platform/latest/html/machine_configuration/mco-update-boot-skew-mgmt for additional details." - - name: ocl-build-failure - rules: - - alert: OCLBuildFailed - expr: | - increase(ocl_build_total{state="failed"}[5m]) > 0 - for: 1m - labels: - namespace: openshift-machine-config-operator - severity: warning - annotations: - summary: "On-Cluster Layering build failed for pool {{ $labels.pool }}" - description: "An OCL image build has failed for MachineConfigPool {{ $labels.pool }}. Nodes in this pool will not receive updated layered images until the build succeeds. Check the machine-os-builder pod logs for details." - runbook_url: https://github.com/openshift/runbooks/blob/master/alerts/machine-config-operator/OCLBuildFailed.md - - name: ocl-build-degraded - rules: - - alert: OCLBuildDegraded - expr: | - ocl_build_state{state="failed"} == 1 - for: 10m - labels: - namespace: openshift-machine-config-operator - severity: warning - annotations: - summary: "On-Cluster Layering build is degraded for pool {{ $labels.pool }}" - description: "OCL build for MachineConfigPool {{ $labels.pool }} is in a persistent failed state. Check the MachineOSBuild status conditions and machine-os-builder pod logs." - runbook_url: https://github.com/openshift/runbooks/blob/master/alerts/machine-config-operator/OCLBuildDegraded.md - - name: ocl-image-push-failure - rules: - - alert: OCLImagePushFailed - expr: | - increase(ocl_image_push_total{state="failed"}[5m]) > 0 - for: 10m - labels: - namespace: openshift-machine-config-operator - severity: warning - annotations: - summary: "On-Cluster Layering image push failed for pool {{ $labels.pool }}" - description: "An OCL image push failed for MachineConfigPool {{ $labels.pool }}. The build job completed but the image was not pushed to the registry. Check registry credentials and machine-os-builder pod logs." - runbook_url: https://github.com/openshift/runbooks/blob/master/alerts/machine-config-operator/OCLImagePushFailed.md + description: "Boot image skew enforcement mode is set to None. When scaling up, new nodes may be provisioned with older boot images that could introduce compatibility issues. Consider manually updating boot images to match the cluster version. Please refer to docs at https://docs.redhat.com/en/documentation/openshift_container_platform/latest/html/machine_configuration/mco-update-boot-skew-mgmt for additional details." --- apiVersion: monitoring.coreos.com/v1 kind: PrometheusRule diff --git a/manifests/machineconfigcontroller/install-config-role.yaml b/manifests/machineconfigcontroller/install-config-role.yaml deleted file mode 100644 index 21c7eab702..0000000000 --- a/manifests/machineconfigcontroller/install-config-role.yaml +++ /dev/null @@ -1,10 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: Role -metadata: - name: machine-config-controller-install-config - namespace: kube-system -rules: -- apiGroups: [""] - resources: ["configmaps"] - resourceNames: ["cluster-config-v1"] - verbs: ["get"] diff --git a/manifests/machineconfigcontroller/install-config-rolebinding.yaml b/manifests/machineconfigcontroller/install-config-rolebinding.yaml deleted file mode 100644 index e821ef96e1..0000000000 --- a/manifests/machineconfigcontroller/install-config-rolebinding.yaml +++ /dev/null @@ -1,13 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding -metadata: - name: machine-config-controller-install-config - namespace: kube-system -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: Role - name: machine-config-controller-install-config -subjects: -- kind: ServiceAccount - namespace: {{.TargetNamespace}} - name: machine-config-controller diff --git a/pkg/apihelpers/apihelpers.go b/pkg/apihelpers/apihelpers.go index 31809a182f..d62318e241 100644 --- a/pkg/apihelpers/apihelpers.go +++ b/pkg/apihelpers/apihelpers.go @@ -439,12 +439,7 @@ func AreMCGeneratingSubControllersCompletedForPool(crcLister func(labels.Selecto return fmt.Errorf("status for KubeletConfig %s is being reported for %d, expecting it for %d", mck.ObjectMeta.Name, mck.Status.ObservedGeneration, mck.Generation) } - if len(mck.Status.Conditions) == 0 { - return fmt.Errorf("KubeletConfig has not completed") - } - lastCondition := mck.Status.Conditions[len(mck.Status.Conditions)-1] - if !((lastCondition.Type == mcfgv1.KubeletConfigAccepted && lastCondition.Status == corev1.ConditionTrue) || - lastCondition.Type == mcfgv1.KubeletConfigSuccess) { // backwards compatibility + if mck.Status.Conditions[len(mck.Status.Conditions)-1].Type != mcfgv1.KubeletConfigSuccess { return fmt.Errorf("KubeletConfig has not completed") } } diff --git a/pkg/controller/bootimage/ami.go b/pkg/controller/bootimage/ami.go index c6a5a91d72..dc1cde0956 100644 --- a/pkg/controller/bootimage/ami.go +++ b/pkg/controller/bootimage/ami.go @@ -1882,45 +1882,5 @@ var AllowedAMIs = sets.New( "ami-0f4f5b97291d204aa", "ami-0f518858c9c61520f", "ami-0f51e2436e0ed93ef", "ami-0f70a80435f95369f", "ami-0f78698c377b198dd", "ami-0f8259fb88cd731a9", "ami-0f85f0de1b8ffb89f", "ami-0f8a95d604562562b", "ami-0f8e1456e093c2396", "ami-0f9c2cdfa1e965dcc", "ami-0f9d01760c1809e35", "ami-0fa80d356c323288c", "ami-0faffb65920395904", "ami-0fb9513e96a103d58", "ami-0fcbfc54371e9351a", - "ami-0fe270d47b42d792f", "ami-0fedec19a27a5eb0d", "ami-0ff0fd0ccdb0d31c2", "ami-0ff7ca07d83e23777", "ami-0001b45ada7d2dc51", - "ami-00159303d0698a4e3", "ami-001eff5ce74306328", "ami-002a19eea3002d594", "ami-003f9933b627323b8", "ami-00543bb0095fa7d23", - "ami-00667f67a54be771a", "ami-008a7997379c73959", "ami-00aaf7a70b4625181", "ami-00af5f0251f9d393f", "ami-00bceb1d4863de8d5", - "ami-00edb1c1d604ca98a", "ami-00f300f51af36e885", "ami-01086887bd3c994e1", "ami-013d71691d6282b85", "ami-01402f6fc7fa6e6dd", - "ami-016bcb89d328ec1ac", "ami-016e6fabf9dda07aa", "ami-01734737bbfbe74a1", "ami-018a94206d329a769", "ami-01b8ebd9b3d74d74c", - "ami-01c8bfb2576fb7da8", "ami-01d48aa6b50f9d0df", "ami-01f10c1ca2ec66aa1", "ami-02134e8468c69e038", "ami-0221fbcd56aa31a7a", - "ami-02264d13cc379c400", "ami-026c3565b2e140a8f", "ami-0286cff0da9d1c1c4", "ami-02d27a05c7b8d13df", "ami-02e98f28520c61cd3", - "ami-02ffc396c2432d05f", "ami-03038bbbf3c588c37", "ami-030623e6ced67aefc", "ami-030ca5bf92778d65f", "ami-0315581cb0f47f844", - "ami-03362167b2060e845", "ami-034a47eb2ad3e3ffd", "ami-03572bea3f4251a57", "ami-035c3042668af014c", "ami-035e211736d96d4a9", - "ami-035fc3102b495e061", "ami-038087b95229af209", "ami-038a860637181b890", "ami-0399f6912ea26e33c", "ami-03b58703d4e9ea61a", - "ami-03c5c13a072aae969", "ami-03d56fdb1a727352c", "ami-03e766cc0ac5a9d5e", "ami-03fcff06b8c5ac019", "ami-03fe2813461e69675", - "ami-04322409852db2c85", "ami-0450d6616ff432cfd", "ami-0477b3b24fd452453", "ami-04a4d537de220f7e6", "ami-04d475d929cabfc98", - "ami-04d6c62a05dc3820a", "ami-04d95d787cc499e7b", "ami-04deeaada42d956a3", "ami-04f22d56acba8a165", "ami-04fe481e7565ae103", - "ami-0507f999c00da7b28", "ami-0537c012fb351d6a0", "ami-0540d29d57768211f", "ami-054a256518f6779b1", "ami-05754d266e357951a", - "ami-059f37640c6141981", "ami-05b3066122ae9e9ec", "ami-05b5ed005b5082f42", "ami-05b949c2a121f003b", "ami-05c37201878e36dac", - "ami-05c6c3d4e9396d633", "ami-061e04994c7f50e3d", "ami-06259340f24c14377", "ami-0625d6961800b66ab", "ami-0648fefdfdfd37772", - "ami-065229dcdeea5f9b6", "ami-06593313b37053df5", "ami-066c6f5fba5f747a8", "ami-067f68ab29b40d089", "ami-06837ef5f71b2333b", - "ami-06a0a6cdc31d687b4", "ami-06b4716233fe63dc9", "ami-06e5d42796bd6f528", "ami-06fd07583064ed540", "ami-07031017151259e76", - "ami-071372b5430c069a2", "ami-0720c264eaffd3628", "ami-0738a052f29ebb8d4", "ami-0754162aa1a6d247e", "ami-0754d7722527cf959", - "ami-0757ca703a44448d7", "ami-076ce220d3f44fa4f", "ami-076e084cea4ec4903", "ami-0783bed23a0fc3ca6", "ami-07842bb8f69d670e1", - "ami-07c15edaa047a52c1", "ami-07cb628f267b263d1", "ami-07d5097d7ef637209", "ami-07f067e13ec1f3eb6", "ami-083d0fa187efa39e3", - "ami-084bd2f45461b2641", "ami-084f80cae5527784a", "ami-0858193e4c8ee5b62", "ami-087614ea800b0b80a", "ami-087ff0fa7fd5aa606", - "ami-088ea81e2a08b663e", "ami-08a4fe7bbdf941fc9", "ami-08ae75477cfec863f", "ami-08b9fb58c26d81f5d", "ami-08c4e237a48e8a24d", - "ami-08d3e5ac783f96689", "ami-08d5a47fb2f53f23a", "ami-08dd96b9cd7af93b9", "ami-08fabbfdb4f9e6482", "ami-090627c0bd0ce09e3", - "ami-090e98376b008c24c", "ami-0918a6de7debcd942", "ami-09493294918f32a0c", "ami-094e87d0b8aab28df", "ami-0980843632a1c3a66", - "ami-09d49112a1306f262", "ami-0a44f8809b6269f6d", "ami-0a4c225c885068069", "ami-0a651d826444a4948", "ami-0a7020eb754a9f6fc", - "ami-0a7afa68ca22609dd", "ami-0a7b15fc279056718", "ami-0a85236299582ce82", "ami-0a90a0dc9913213aa", "ami-0aba7be9d548d8831", - "ami-0ae75c9607a8862d8", "ami-0aee926ea72d3b04c", "ami-0af5f9e96ff66bdf5", "ami-0b5fba6fa0c7197d8", "ami-0b8305344e0fc7d2d", - "ami-0ba2883e0cf3e3558", "ami-0bba320960f966f20", "ami-0bd752fae0b2ac2a4", "ami-0bddd3545406608aa", "ami-0be21023c7da97e39", - "ami-0beed6ca33a1ae42d", "ami-0bfd310dade0d036f", "ami-0c19d58cb75d733c0", "ami-0c231f4816ba0776a", "ami-0c3faadd1797e3868", - "ami-0c500dfe48597e6f6", "ami-0c78f64d9a885c173", "ami-0c8487cb7aca26300", "ami-0cab770415626f477", "ami-0cb61ac0cd757c89c", - "ami-0cbb81ed4429c0503", "ami-0ccd894c5c2c7c45c", "ami-0cd956ca21ed98150", "ami-0cf685d7c4e2f115b", "ami-0d05715541d4175e7", - "ami-0d1687e928755427e", "ami-0d16b2d6a474b4360", "ami-0d1755bc956801be0", "ami-0d320aa9cc1633024", "ami-0d598411cb2c6a0e5", - "ami-0d6093add7a207886", "ami-0d68739d819850b39", "ami-0d6e91c6dae1b3c98", "ami-0d8a13644e176f1f8", "ami-0d9e44cb94e3e2314", - "ami-0dc49d202ca3f9193", "ami-0dd185285ca843d25", "ami-0de2f48ef57114708", "ami-0de3372c53819aaaa", "ami-0df26f104866e1a9a", - "ami-0e099dce9627ef699", "ami-0e0ac57dadcaf1ab9", "ami-0e0cf94e34dd227cd", "ami-0e0f89624ae6f6561", "ami-0e38d6bd2282d8072", - "ami-0e3ce644b0d9b5eab", "ami-0e3f9a41df3b903c6", "ami-0e47406ea13abefd8", "ami-0e651950cf2f78a61", "ami-0e6941714240039fa", - "ami-0e6a4ffe9dd1dee56", "ami-0e721586ef04044d2", "ami-0e75465363d8378f0", "ami-0e79be45f2303f422", "ami-0e9a0f9e1a4c49b92", - "ami-0ec2f94496976cb5f", "ami-0f186d1a862e47c8f", "ami-0f480726bfb767807", "ami-0f5633a3440704a9d", "ami-0f567edf0859e3b39", - "ami-0f5b937ac530500da", "ami-0f752ff59c8b0d39f", "ami-0f7add16535539145", "ami-0f99bc65b81da18b0", "ami-0fb8045529e752a6f", - "ami-0fc49ff64e479b1f5", "ami-0fe15e383af1e71d4", "ami-0feb18654dafb55c6", "ami-0fed99e2bf80aea18", + "ami-0fe270d47b42d792f", "ami-0fedec19a27a5eb0d", "ami-0ff0fd0ccdb0d31c2", "ami-0ff7ca07d83e23777", ) diff --git a/pkg/controller/bootimage/platform_helpers.go b/pkg/controller/bootimage/platform_helpers.go index b13385cd42..89e9904db8 100644 --- a/pkg/controller/bootimage/platform_helpers.go +++ b/pkg/controller/bootimage/platform_helpers.go @@ -233,7 +233,7 @@ func reconcileAWSProviderSpec(streamData *stream.Stream, arch string, _ *osconfi return true, false, newProviderSpec, rhcosVersion, nil } -func reconcileVSphereProviderSpec(streamData *stream.Stream, arch string, infra *osconfigv1.Infrastructure, providerSpec *machinev1beta1.VSphereMachineProviderSpec, _ string, kubeClient clientset.Interface) (bool, bool, *machinev1beta1.VSphereMachineProviderSpec, string, error) { +func reconcileVSphereProviderSpec(streamData *stream.Stream, arch string, infra *osconfigv1.Infrastructure, providerSpec *machinev1beta1.VSphereMachineProviderSpec, _ string, secretClient clientset.Interface) (bool, bool, *machinev1beta1.VSphereMachineProviderSpec, string, error) { if infra.Spec.PlatformSpec.VSphere == nil { klog.Warningf("Reconcile skipped: VSphere field is nil in PlatformSpec %v", infra.Spec.PlatformSpec) @@ -253,12 +253,12 @@ func reconcileVSphereProviderSpec(streamData *stream.Stream, arch string, infra newProviderSpec := providerSpec.DeepCopy() // Fetch the creds configmap - credsSc, err := kubeClient.CoreV1().Secrets("kube-system").Get(context.TODO(), "vsphere-creds", metav1.GetOptions{}) + credsSc, err := secretClient.CoreV1().Secrets("kube-system").Get(context.TODO(), "vsphere-creds", metav1.GetOptions{}) if err != nil { return false, false, nil, "", fmt.Errorf("failed to fetch vsphere-creds Secret during machineset sync: %w", err) } - newBootImg, patchRequired, err := createNewVMTemplate(streamData, providerSpec, infra, credsSc, kubeClient, arch, artifacts.Release) + newBootImg, patchRequired, err := createNewVMTemplate(streamData, providerSpec, infra, credsSc, arch, artifacts.Release) if err != nil { return false, false, nil, "", err } @@ -266,7 +266,7 @@ func reconcileVSphereProviderSpec(streamData *stream.Stream, arch string, infra // If patch is required, marshal the new providerspec into the machineset if patchRequired { // Ensure the ignition stub is the minimum acceptable spec required for boot image updates - if err := upgradeStubIgnitionIfRequired(providerSpec.UserDataSecret.Name, kubeClient); err != nil { + if err := upgradeStubIgnitionIfRequired(providerSpec.UserDataSecret.Name, secretClient); err != nil { return false, false, nil, "", err } newProviderSpec.Template = newBootImg diff --git a/pkg/controller/bootimage/vsphere_helpers.go b/pkg/controller/bootimage/vsphere_helpers.go index fd8d36342f..fc835c9528 100644 --- a/pkg/controller/bootimage/vsphere_helpers.go +++ b/pkg/controller/bootimage/vsphere_helpers.go @@ -26,11 +26,8 @@ import ( osconfigv1 "github.com/openshift/api/config/v1" machinev1beta1 "github.com/openshift/api/machine/v1beta1" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - clientset "k8s.io/client-go/kubernetes" "k8s.io/klog/v2" "k8s.io/utils/ptr" - "sigs.k8s.io/yaml" "github.com/openshift/machine-config-operator/pkg/controller/bootimage/cache" ) @@ -205,31 +202,10 @@ func findAllRequiredResources(ctx context.Context, finder *find.Finder, provider } else { return nil, fmt.Errorf("finder had error: %w", err) } - } else if !isInFolder(vr.existingVM, vr.folder) { - // Name match outside the workspace folder: a customer-managed VM, not the one MCO manages. - // Leave it untouched so the caller creates a fresh template instead of swapping it out. - klog.Infof("VM %s exists outside the expected workspace folder %s; leaving the customer-managed VM untouched", name, providerSpec.Workspace.Folder) - vr.existingVM = nil } return &vr, nil } -// isInFolder reports whether vm is a direct child of folder. govmomi's Finder.VirtualMachine does a -// global, recursive, name-based search of the entire vCenter inventory with no folder scoping, so a -// name match alone doesn't mean the VM is the one MCO manages. MCO always creates/imports templates -// directly into providerSpec.Workspace.Folder, so anything else found by name is customer-managed and -// must be left untouched. -// -// A nil folder (the workspace folder itself couldn't be resolved) trusts the match instead of -// rejecting it: verifying locality isn't a prerequisite for this function's caller, and anything that -// actually needs to write to vSphere re-resolves the same folder and hard-fails there if it's broken. -func isInFolder(vm *object.VirtualMachine, folder *object.Folder) bool { - if folder == nil { - return true - } - return path.Dir(vm.InventoryPath) == folder.InventoryPath -} - // getDiskTypeFromExistingVM inspects the given VM's disk backing configuration and returns its disk provisioning type (thin, thick, eagerZeroedThick). func getDiskTypeFromExistingVM(vmMo mo.VirtualMachine) string { diskType := "" @@ -253,153 +229,6 @@ func getDiskTypeFromExistingVM(vmMo mo.VirtualMachine) string { return diskType } -const ( - installConfigConfigMapName = "cluster-config-v1" - installConfigConfigMapKey = "install-config" -) - -// installConfig mirrors the subset of the install-config schema needed to recover the vSphere disk -// provisioning type. This is only ever set at install time and is not persisted anywhere else the -// operator can read it (VSphereMachineProviderSpec, Infrastructure, failure domain topology all omit it). -type installConfig struct { - Platform struct { - VSphere struct { - DiskType string `json:"diskType"` - } `json:"vsphere"` - } `json:"platform"` -} - -// getDiskTypeFromInstallConfig reads platform.vsphere.diskType from the cluster's install-config -// configmap. Used for brand-new failure domains, where there is no existing template VM to inspect -// via getDiskTypeFromExistingVM. Returns "" (no error) if the installer left diskType unset, which is -// a legitimate value: getCISP treats "" as "use the datastore's default storage policy." -func getDiskTypeFromInstallConfig(ctx context.Context, kubeClient clientset.Interface) (string, error) { - cm, err := kubeClient.CoreV1().ConfigMaps("kube-system").Get(ctx, installConfigConfigMapName, metav1.GetOptions{}) - if err != nil { - return "", fmt.Errorf("failed to fetch %s configmap: %w", installConfigConfigMapName, err) - } - - raw, ok := cm.Data[installConfigConfigMapKey] - if !ok { - return "", fmt.Errorf("%s configmap has no %q key", installConfigConfigMapName, installConfigConfigMapKey) - } - - var ic installConfig - if err := yaml.Unmarshal([]byte(raw), &ic); err != nil { - return "", fmt.Errorf("failed to parse install-config: %w", err) - } - - return ic.Platform.VSphere.DiskType, nil -} - -// resolveExistingTemplateVM locates the template VM for a failure domain using a two-step lookup: providerSpec.Template first -// then the infra computed name. If neither exists in the workspace folder, it creates a fresh template from the OVA and -// signals the caller via created=true. resolvedName is the vSphere name of the VM that was actually found, which may differ -// from name when the VM was located via providerSpec.Template; callers must use resolvedName (not name) for divergence checks. -// -// A name match outside providerSpec.Workspace.Folder is never treated as the existing template: govmomi's finder searches -// the entire vCenter inventory by name, so a match elsewhere may be a customer-managed VM MCO must not touch (see isInFolder). -// If the workspace folder itself can't be resolved, this check is skipped (logging a warning) rather than failing the -// reconcile: verifying it isn't this function's job — it's a locality check for classification, not a prerequisite for -// action. -func resolveExistingTemplateVM( - ctx context.Context, - finder *find.Finder, - providerSpec *machinev1beta1.VSphereMachineProviderSpec, - failureDomain osconfigv1.VSpherePlatformFailureDomainSpec, - streamData *stream.Stream, - client *govmomi.Client, - tagManager *tags.Manager, - kubeClient clientset.Interface, - infraID, arch string, -) (vm *object.VirtualMachine, resolvedName string, created bool, err error) { - - computedName := fmt.Sprintf("%s-rhcos-%s", infraID, failureDomain.Name) - var notFoundErr *find.NotFoundError - - workspaceFolder, folderErr := finder.Folder(ctx, providerSpec.Workspace.Folder) - if folderErr != nil { - klog.Warningf("failed to resolve workspace folder %q; cannot verify template VM locality this reconcile, proceeding with name-based lookup only: %v", providerSpec.Workspace.Folder, folderErr) - } - - // Check providerSpec.Template first so a freshly-added failure domain whose MachineSet - // already has a valid template doesn't fail just because the infra computed name isn't a match. - if providerSpec.Template != "" && providerSpec.Template != computedName { - tmplVM, tmplErr := finder.VirtualMachine(ctx, providerSpec.Template) - switch { - case tmplErr == nil && isInFolder(tmplVM, workspaceFolder): - return tmplVM, providerSpec.Template, false, nil - case tmplErr == nil: - klog.Infof("providerSpec.Template %s exists outside the expected workspace folder %s; leaving the customer-managed VM untouched and falling back to computed name %s", providerSpec.Template, providerSpec.Workspace.Folder, computedName) - case errors.As(tmplErr, ¬FoundErr): - klog.Infof("providerSpec.Template %s not found in vSphere; falling back to computed name %s", providerSpec.Template, computedName) - default: - klog.Warningf("Unexpected error looking up providerSpec.Template %s: %v; falling back to computed name %s", providerSpec.Template, tmplErr, computedName) - } - } - - vm, err = finder.VirtualMachine(ctx, computedName) - switch { - case err == nil && isInFolder(vm, workspaceFolder): - return vm, computedName, false, nil - case err == nil: - klog.Infof("VM %s exists outside the expected workspace folder %s; leaving the customer-managed VM untouched and creating a fresh template", computedName, providerSpec.Workspace.Folder) - case !errors.As(err, ¬FoundErr): - return nil, "", false, fmt.Errorf("finder had error: %w", err) - } - - // Computed name not found in the workspace folder — either nothing exists there, or the name is - // held by a customer-managed VM elsewhere in the inventory that must be left alone. Either way, - // check for a rollback VM left by a prior mid-swap crash before creating a fresh template. - oldTempName := atomicTempName("mco-old", computedName) - oldVM, oldErr := finder.VirtualMachine(ctx, oldTempName) - if oldErr != nil { - if !errors.As(oldErr, ¬FoundErr) { - return nil, "", false, fmt.Errorf("error looking up rollback VM %s: %w", oldTempName, oldErr) - } - // No template exists anywhere — failure domain is newly added; create from OVA. - klog.Infof("No existing template found for failure domain %s; creating new template %s from OVA", failureDomain.Name, computedName) - if len(computedName) > 80 { - return nil, "", false, fmt.Errorf("length of VM template name `%s` exceeds the permitted limit of 80 characters", computedName) - } - ova, ovaErr := streamData.QueryDisk(arch, "vmware", "ova") - if ovaErr != nil { - return nil, "", false, ovaErr - } - ovaPath, ovaErr := cache.DownloadOva(ova) - if ovaErr != nil { - return nil, "", false, fmt.Errorf("failed to download %s: %w", ova.Location, ovaErr) - } - // No existing VM to inspect for this brand-new failure domain; the only other place - // disk provisioning type is recorded is the install-time install-config. - diskType, diskTypeErr := getDiskTypeFromInstallConfig(ctx, kubeClient) - if diskTypeErr != nil { - return nil, "", false, fmt.Errorf("failed to determine disk provisioning type for new template %s: %w", computedName, diskTypeErr) - } - if createErr := createNewVMTemplateWithNameForFailureDomain(ctx, providerSpec, failureDomain, finder, client, tagManager, computedName, ovaPath, infraID, diskType); createErr != nil { - return nil, "", false, createErr - } - return nil, computedName, true, nil - } - - // Rollback: restore the old template renamed away during a crashed atomic swap. - klog.Infof("Recovering from mid-swap crash: renaming %s back to %s", oldTempName, computedName) - renameTask, renameErr := oldVM.Rename(ctx, computedName) - if renameErr != nil { - return nil, "", false, fmt.Errorf("failed to initiate rollback rename of %s to %s: %w", oldTempName, computedName, renameErr) - } - if renameErr = renameTask.Wait(ctx); renameErr != nil { - return nil, "", false, fmt.Errorf("failed to complete rollback rename of %s to %s: %w", oldTempName, computedName, renameErr) - } - // Confirm the rollback succeeded; any stale mco-tmp-* VM will be cleaned up by - // createNewVMTemplateWithNameForFailureDomain if an update is subsequently needed. - vm, err = finder.VirtualMachine(ctx, computedName) - if err != nil { - return nil, "", false, fmt.Errorf("failed to fetch rolled-back template %s: %w", computedName, err) - } - return vm, computedName, false, nil -} - // atomicTempName returns a short, fixed-length (17-char) VM name for use during atomic template // swaps. The hash makes it deterministic and unique per final name, bypassing the 80-char limit. func atomicTempName(prefix, name string) string { @@ -670,10 +499,11 @@ func getClientsFromServerURL(ctx context.Context, server, username, password str } // Creates a Template VM which has the relevant OVA/OVF file -func createNewVMTemplate(streamData *stream.Stream, providerSpec *machinev1beta1.VSphereMachineProviderSpec, infra *osconfigv1.Infrastructure, credsSc *corev1.Secret, kubeClient clientset.Interface, arch, release string) (string, bool, error) { +func createNewVMTemplate(streamData *stream.Stream, providerSpec *machinev1beta1.VSphereMachineProviderSpec, infra *osconfigv1.Infrastructure, credsSc *corev1.Secret, arch, release string) (string, bool, error) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() + var name string for _, vcenter := range infra.Spec.PlatformSpec.VSphere.VCenters { if vcenter.Server != providerSpec.Workspace.Server { continue @@ -716,12 +546,36 @@ func createNewVMTemplate(streamData *stream.Stream, providerSpec *machinev1beta1 } finder = finder.SetDatacenter(datacenter) - existingTemplateVM, resolvedName, created, err := resolveExistingTemplateVM(ctx, finder, providerSpec, failureDomain, streamData, client, tagManager, kubeClient, infraID, arch) + name = fmt.Sprintf("%s-rhcos-%s", infraID, failureDomain.Name) + + existingTemplateVM, err := finder.VirtualMachine(ctx, name) if err != nil { - return "", false, err - } - if created { - return resolvedName, true, nil + if _, ok := err.(*find.NotFoundError); !ok { + return "", false, fmt.Errorf("finder had error: %w", err) + } + // Template not found; update likely crashed between the two rename steps of a prior + // atomic swap. Check for the old template under its rollback name and restore it + // so the next reconciliation loop can proceed normally. + oldTempName := atomicTempName("mco-old", name) + oldVM, oldErr := finder.VirtualMachine(ctx, oldTempName) + if oldErr != nil { + return "", false, fmt.Errorf("template %s not found : %w and no rollback VM %s present: %w", name, err, oldTempName, oldErr) + } + klog.Infof("Recovering from mid-swap crash: renaming %s back to %s", oldTempName, name) + renameTask, renameErr := oldVM.Rename(ctx, name) + if renameErr != nil { + return "", false, fmt.Errorf("failed to initiate rollback rename of %s to %s: %w", oldTempName, name, renameErr) + } + if renameErr = renameTask.Wait(ctx); renameErr != nil { + return "", false, fmt.Errorf("failed to complete rollback rename of %s to %s: %w", oldTempName, name, renameErr) + } + // Fresh fetch by name to confirm the rollback succeeded and get a clean reference. + // Any stale mco-tmp-* VM will be cleaned up by createNewVMTemplateWithNameForFailureDomain + // if an update is subsequently needed. + existingTemplateVM, err = finder.VirtualMachine(ctx, name) + if err != nil { + return "", false, fmt.Errorf("failed to fetch rolled-back template %s: %w", name, err) + } } var vmMo mo.VirtualMachine @@ -730,58 +584,50 @@ func createNewVMTemplate(streamData *stream.Stream, providerSpec *machinev1beta1 return "", false, fmt.Errorf("unable to extract properties from existing Template VM: %w", err) } - if vmMo.Summary.Config.Product == nil { - return "", false, fmt.Errorf("unable to determine RHCOS version of virtual machine: %s", providerSpec.Template) - } + if vmMo.Summary.Config.Product != nil { + templateProductVersion := vmMo.Summary.Config.Product.Version + if templateProductVersion == "" { + return "", false, fmt.Errorf("unable to determine RHCOS version of virtual machine: %s", providerSpec.Template) + } - templateProductVersion := vmMo.Summary.Config.Product.Version - if templateProductVersion == "" { - return "", false, fmt.Errorf("unable to determine RHCOS version of virtual machine: %s", providerSpec.Template) - } + if templateProductVersion != release { + klog.Infof("Existing RHCOS v%s does not match current RHCOS v%s. Starting reconciliation process.", templateProductVersion, release) - if templateProductVersion != release { - klog.Infof("Existing RHCOS v%s does not match current RHCOS v%s. Starting reconciliation process.", templateProductVersion, release) + // Find and download the relevant OVA file + ova, err := streamData.QueryDisk(arch, "vmware", "ova") + if err != nil { + return "", false, err + } - // Find and download the relevant OVA file - ova, err := streamData.QueryDisk(arch, "vmware", "ova") - if err != nil { - return "", false, err - } + ovaPath, err := cache.DownloadOva(ova) + if err != nil { + return "", false, fmt.Errorf("failed to download %s: %w", ova.Location, err) + } - ovaPath, err := cache.DownloadOva(ova) - if err != nil { - return "", false, fmt.Errorf("failed to download %s: %w", ova.Location, err) - } + if len(name) > 80 { + return "", false, fmt.Errorf("length of VM template name `%s` exceeds the permitted limit of 80 characters", name) + } - if len(resolvedName) > 80 { - return "", false, fmt.Errorf("length of VM template name `%s` exceeds the permitted limit of 80 characters", resolvedName) - } + diskType := getDiskTypeFromExistingVM(vmMo) - diskType := getDiskTypeFromExistingVM(vmMo) + err = createNewVMTemplateWithNameForFailureDomain(ctx, providerSpec, failureDomain, finder, client, tagManager, name, ovaPath, infraID, diskType) + if err != nil { + return "", false, err + } + return name, true, nil + } - err = createNewVMTemplateWithNameForFailureDomain(ctx, providerSpec, failureDomain, finder, client, tagManager, resolvedName, ovaPath, infraID, diskType) - if err != nil { - return "", false, err + klog.Infof("Existing RHCOS v%s does match current RHCOS v%s. Skipping reconciliation process using govmomi.", templateProductVersion, release) + if providerSpec.Template != name { + klog.Infof("ProviderSpec template name: %s has diverged from the VM Template of name: %s that exists in VSphere. Reconciling the name change.", providerSpec.Template, name) + return name, true, nil } - return resolvedName, true, nil - } - klog.Infof("Existing RHCOS v%s does match current RHCOS v%s. Skipping reconciliation process using govmomi.", templateProductVersion, release) - if providerSpec.Template != resolvedName { - klog.Infof("ProviderSpec template name: %s has diverged from the VM Template of name: %s that exists in VSphere. Reconciling the name change.", providerSpec.Template, resolvedName) - return resolvedName, true, nil + } else { + return "", false, fmt.Errorf("unable to determine RHCOS version of virtual machine: %s", providerSpec.Template) } - return resolvedName, false, nil } } - workspaceDetails := fmt.Sprintf("server: %s, datacenter: %s, datastore: %s, resourcePool: %s", - providerSpec.Workspace.Server, providerSpec.Workspace.Datacenter, providerSpec.Workspace.Datastore, providerSpec.Workspace.ResourcePool) - // vmGroup only applies to HostGroup-zonal failure domains (VSphereFailureDomainZoneAffinity.Type - // == HostGroup; see the vmGroup derivation above) — omit it here when unset rather than implying - // it's a universal match criterion for every failure domain type (e.g. ComputeCluster-zoned ones). - if providerSpec.Workspace.VMGroup != "" { - workspaceDetails += fmt.Sprintf(", vmGroup: %s", providerSpec.Workspace.VMGroup) - } - return "", false, fmt.Errorf("providerSpec workspace (%s) does not match any vCenter/failure domain in the Infrastructure object", workspaceDetails) + return "", false, nil } diff --git a/pkg/controller/bootimage/vsphere_helpers_test.go b/pkg/controller/bootimage/vsphere_helpers_test.go deleted file mode 100644 index 1f77db8bb7..0000000000 --- a/pkg/controller/bootimage/vsphere_helpers_test.go +++ /dev/null @@ -1,114 +0,0 @@ -package bootimage - -import ( - "testing" - - "github.com/vmware/govmomi/object" - "github.com/vmware/govmomi/vim25/types" - - machinev1beta1 "github.com/openshift/api/machine/v1beta1" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - osconfigv1 "github.com/openshift/api/config/v1" -) - -// TestCreateNewVMTemplate_NoMatchingFailureDomain verifies that when a MachineSet's -// providerSpec.Workspace doesn't match any vCenter/failure domain in the Infrastructure object, -// createNewVMTemplate returns a descriptive error instead of silently no-op'ing. This is the -// degrade-on-no-match behavior added in "bootimage: degrade when vsphere fd not found" — it never -// reaches getClientsFromServerURL (no real vCenter connectivity needed), since the outer loop over -// infra.Spec.PlatformSpec.VSphere.VCenters has nothing to match against. -func TestCreateNewVMTemplate_NoMatchingFailureDomain(t *testing.T) { - providerSpec := &machinev1beta1.VSphereMachineProviderSpec{ - Workspace: &machinev1beta1.Workspace{ - Server: "vcenter.example.com", - Datacenter: "dc1", - Datastore: "datastore1", - ResourcePool: "/dc1/host/cluster1/Resources", - }, - } - - infra := &osconfigv1.Infrastructure{ - Spec: osconfigv1.InfrastructureSpec{ - PlatformSpec: osconfigv1.PlatformSpec{ - VSphere: &osconfigv1.VSpherePlatformSpec{ - // Deliberately empty: no vCenters/failure domains for providerSpec.Workspace - // to match against. - }, - }, - }, - } - - resolvedName, patchRequired, err := createNewVMTemplate(nil, providerSpec, infra, nil, nil, "x86_64", "9.6.20260210-0") - - require.Error(t, err) - assert.Contains(t, err.Error(), "does not match any vCenter/failure domain") - assert.Contains(t, err.Error(), "vcenter.example.com") - assert.Empty(t, resolvedName) - assert.False(t, patchRequired) -} - -func newTestVM(inventoryPath string) *object.VirtualMachine { - vm := object.NewVirtualMachine(nil, types.ManagedObjectReference{}) - vm.InventoryPath = inventoryPath - return vm -} - -func newTestFolder(inventoryPath string) *object.Folder { - folder := object.NewFolder(nil, types.ManagedObjectReference{}) - folder.InventoryPath = inventoryPath - return folder -} - -// TestIsInFolder verifies the folder-scoping check used to distinguish MCO-managed template VMs -// from customer-managed VMs that merely share a name. govmomi's finder searches by name across the -// entire vCenter inventory, so a name match alone doesn't guarantee the VM lives where MCO expects -// (providerSpec.Workspace.Folder) — only a direct child of that folder counts as MCO-owned. -func TestIsInFolder(t *testing.T) { - workspaceFolder := newTestFolder("/dc1/vm/openshift4-folder") - - tests := []struct { - name string - vm *object.VirtualMachine - folder *object.Folder - want bool - }{ - { - name: "direct child of workspace folder", - vm: newTestVM("/dc1/vm/openshift4-folder/infra-rhcos-fd1"), - folder: workspaceFolder, - want: true, - }, - { - name: "sibling folder", - vm: newTestVM("/dc1/vm/customer-folder/infra-rhcos-fd1"), - folder: workspaceFolder, - want: false, - }, - { - name: "directly under the datacenter's default vm folder", - vm: newTestVM("/dc1/vm/infra-rhcos-fd1"), - folder: workspaceFolder, - want: false, - }, - { - name: "nested subfolder beneath the workspace folder", - vm: newTestVM("/dc1/vm/openshift4-folder/nested/infra-rhcos-fd1"), - folder: workspaceFolder, - want: false, - }, - { - name: "nil folder (workspace folder unresolved) trusts the match", - vm: newTestVM("/dc1/vm/customer-folder/infra-rhcos-fd1"), - folder: nil, - want: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.want, isInFolder(tt.vm, tt.folder)) - }) - } -} diff --git a/pkg/controller/build/ocl_events.go b/pkg/controller/build/ocl_events.go deleted file mode 100644 index a26c2d9fde..0000000000 --- a/pkg/controller/build/ocl_events.go +++ /dev/null @@ -1,181 +0,0 @@ -package build - -import ( - "fmt" - - mcfgv1 "github.com/openshift/api/machineconfiguration/v1" - batchv1 "k8s.io/api/batch/v1" - corev1 "k8s.io/api/core/v1" - "k8s.io/client-go/tools/record" -) - -// Event types for OCL processes -const ( - // Build lifecycle events - EventBuildStarted = "BuildStarted" - EventBuildPreparing = "BuildPreparing" - EventBuildBuilding = "BuildBuilding" - EventBuildCompleted = "BuildCompleted" - EventBuildFailed = "BuildFailed" - EventBuildInterrupted = "BuildInterrupted" - EventBuildDeleted = "BuildDeleted" - - // Job events - EventJobCreated = "JobCreated" - EventJobStarted = "JobStarted" - EventJobCompleted = "JobCompleted" - EventJobFailed = "JobFailed" - EventJobDeleted = "JobDeleted" - - // Config events - EventConfigReconciled = "ConfigReconciled" - EventConfigReconcileFailed = "ConfigReconcileFailed" - EventRebuildRequested = "RebuildRequested" - - // Config deletion events - EventConfigDeleted = "ConfigDeleted" - - // MachineConfigPool events - EventPoolConfigChanged = "PoolConfigChanged" - - // Degradation events - EventBuildDegraded = "BuildDegraded" - EventBuildRecovered = "BuildRecovered" -) - -// OCLEventRecorder wraps the Kubernetes event recorder with OCL-specific event helpers -type OCLEventRecorder struct { - recorder record.EventRecorder -} - -// NewOCLEventRecorder creates a new OCL event recorder -func NewOCLEventRecorder(recorder record.EventRecorder) *OCLEventRecorder { - return &OCLEventRecorder{ - recorder: recorder, - } -} - -// Build lifecycle event recording - -// RecordBuildStarted records when a build starts -func (r *OCLEventRecorder) RecordBuildStarted(mosb *mcfgv1.MachineOSBuild, mosc *mcfgv1.MachineOSConfig) { - r.recorder.Event(mosb, corev1.EventTypeNormal, EventBuildStarted, - fmt.Sprintf("Started build for pool %q with config %q", - mosc.Spec.MachineConfigPool.Name, mosb.Spec.MachineConfig.Name)) -} - -// RecordBuildPreparing records when a build is in the preparing phase -func (r *OCLEventRecorder) RecordBuildPreparing(mosb *mcfgv1.MachineOSBuild, message string) { - r.recorder.Event(mosb, corev1.EventTypeNormal, EventBuildPreparing, - fmt.Sprintf("Preparing build: %s", message)) -} - -// RecordBuildBuilding records when a build transitions to building state -func (r *OCLEventRecorder) RecordBuildBuilding(mosb *mcfgv1.MachineOSBuild) { - r.recorder.Event(mosb, corev1.EventTypeNormal, EventBuildBuilding, - "Build is now in progress") -} - -// RecordBuildCompleted records when a build completes successfully -func (r *OCLEventRecorder) RecordBuildCompleted(mosb *mcfgv1.MachineOSBuild, imagePullspec string) { - r.recorder.Event(mosb, corev1.EventTypeNormal, EventBuildCompleted, - fmt.Sprintf("Build completed successfully, image: %s", imagePullspec)) -} - -// RecordBuildFailed records when a build fails -func (r *OCLEventRecorder) RecordBuildFailed(mosb *mcfgv1.MachineOSBuild) { - r.recorder.Event(mosb, corev1.EventTypeWarning, EventBuildFailed, - fmt.Sprintf("Build failed; see MachineOSBuild %q status conditions for details", mosb.Name)) -} - -// RecordBuildInterrupted records when a build is interrupted -func (r *OCLEventRecorder) RecordBuildInterrupted(mosb *mcfgv1.MachineOSBuild, reason string) { - r.recorder.Event(mosb, corev1.EventTypeWarning, EventBuildInterrupted, - fmt.Sprintf("Build interrupted: %s", reason)) -} - -// RecordBuildDeleted records when a build is deleted -func (r *OCLEventRecorder) RecordBuildDeleted(mosb *mcfgv1.MachineOSBuild, reason string) { - r.recorder.Event(mosb, corev1.EventTypeNormal, EventBuildDeleted, - fmt.Sprintf("Build deleted: %s", reason)) -} - -// Job event recording - -// RecordJobCreated records when a build job is created -func (r *OCLEventRecorder) RecordJobCreated(mosb *mcfgv1.MachineOSBuild, job *batchv1.Job) { - r.recorder.Event(mosb, corev1.EventTypeNormal, EventJobCreated, - fmt.Sprintf("Created build job: %s", job.Name)) -} - -// RecordJobStarted records when a build job starts -func (r *OCLEventRecorder) RecordJobStarted(mosb *mcfgv1.MachineOSBuild, job *batchv1.Job) { - r.recorder.Event(mosb, corev1.EventTypeNormal, EventJobStarted, - fmt.Sprintf("Build job started: %s", job.Name)) -} - -// RecordJobCompleted records when a build job completes -func (r *OCLEventRecorder) RecordJobCompleted(mosb *mcfgv1.MachineOSBuild, job *batchv1.Job) { - r.recorder.Event(mosb, corev1.EventTypeNormal, EventJobCompleted, - fmt.Sprintf("Build job completed: %s", job.Name)) -} - -// RecordJobFailed records when a build job fails -func (r *OCLEventRecorder) RecordJobFailed(mosb *mcfgv1.MachineOSBuild, job *batchv1.Job) { - r.recorder.Event(mosb, corev1.EventTypeWarning, EventJobFailed, - fmt.Sprintf("Build job %q failed; see MachineOSBuild %q status conditions for details", job.Name, mosb.Name)) -} - -// RecordJobDeleted records when a build job is deleted -func (r *OCLEventRecorder) RecordJobDeleted(mosb *mcfgv1.MachineOSBuild, jobName string) { - r.recorder.Event(mosb, corev1.EventTypeNormal, EventJobDeleted, - fmt.Sprintf("Build job deleted: %s", jobName)) -} - -// Config event recording - -// RecordConfigReconciled records when a MachineOSConfig spec change is detected and acted on. -func (r *OCLEventRecorder) RecordConfigReconciled(mosc *mcfgv1.MachineOSConfig) { - r.recorder.Event(mosc, corev1.EventTypeNormal, EventConfigReconciled, - "MachineOSConfig spec change detected and reconciled: new build created or reused") -} - -// RecordConfigReconcileFailed records when a MachineOSConfig spec change could not be reconciled. -func (r *OCLEventRecorder) RecordConfigReconcileFailed(mosc *mcfgv1.MachineOSConfig, err error) { - r.recorder.Event(mosc, corev1.EventTypeWarning, EventConfigReconcileFailed, - fmt.Sprintf("Failed to reconcile MachineOSConfig spec change: %v", err)) -} - -// RecordRebuildRequested records when a rebuild is requested -func (r *OCLEventRecorder) RecordRebuildRequested(mosc *mcfgv1.MachineOSConfig, reason string) { - r.recorder.Event(mosc, corev1.EventTypeNormal, EventRebuildRequested, - fmt.Sprintf("Rebuild requested: %s", reason)) -} - -// RecordConfigDeleted records when a MachineOSConfig is deleted -func (r *OCLEventRecorder) RecordConfigDeleted(mosc *mcfgv1.MachineOSConfig) { - r.recorder.Event(mosc, corev1.EventTypeNormal, EventConfigDeleted, - fmt.Sprintf("MachineOSConfig %q deleted, removing associated builds", mosc.Name)) -} - -// Degradation event recording - -// RecordBuildDegraded records when a build enters degraded state -func (r *OCLEventRecorder) RecordBuildDegraded(mosc *mcfgv1.MachineOSConfig) { - r.recorder.Event(mosc, corev1.EventTypeWarning, EventBuildDegraded, - fmt.Sprintf("Build for pool %q degraded; see MachineOSBuild status conditions for details", mosc.Spec.MachineConfigPool.Name)) -} - -// RecordBuildRecovered records when a build recovers from degraded state -func (r *OCLEventRecorder) RecordBuildRecovered(mosc *mcfgv1.MachineOSConfig) { - r.recorder.Event(mosc, corev1.EventTypeNormal, EventBuildRecovered, - fmt.Sprintf("Build for pool %q recovered from degraded state", mosc.Spec.MachineConfigPool.Name)) -} - -// MachineConfigPool events - -// RecordPoolConfigChanged records when a pool's config changes -func (r *OCLEventRecorder) RecordPoolConfigChanged(mcp *mcfgv1.MachineConfigPool, oldConfig, newConfig string) { - r.recorder.Event(mcp, corev1.EventTypeNormal, EventPoolConfigChanged, - fmt.Sprintf("Rendered config changed from %s to %s", oldConfig, newConfig)) -} diff --git a/pkg/controller/build/ocl_events_test.go b/pkg/controller/build/ocl_events_test.go deleted file mode 100644 index 940af5dd2d..0000000000 --- a/pkg/controller/build/ocl_events_test.go +++ /dev/null @@ -1,235 +0,0 @@ -package build - -import ( - "fmt" - "strings" - "testing" - - mcfgv1 "github.com/openshift/api/machineconfiguration/v1" - batchv1 "k8s.io/api/batch/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/tools/record" -) - -func newTestOCLRecorder(bufSize int) (*OCLEventRecorder, *record.FakeRecorder) { - fake := record.NewFakeRecorder(bufSize) - return NewOCLEventRecorder(fake), fake -} - -func assertEvent(t *testing.T, events chan string, wantType, wantReason string) { - t.Helper() - select { - case event := <-events: - if !strings.HasPrefix(event, wantType+" ") { - t.Errorf("expected event type %q, got: %s", wantType, event) - } - if !strings.Contains(event, wantReason) { - t.Errorf("expected event reason %q, got: %s", wantReason, event) - } - default: - t.Errorf("expected event %q %q but none was recorded", wantType, wantReason) - } -} - -func assertNoEvent(t *testing.T, events chan string) { - t.Helper() - select { - case event := <-events: - t.Errorf("expected no event but got: %s", event) - default: - } -} - -func testMOSB(name string) *mcfgv1.MachineOSBuild { - return &mcfgv1.MachineOSBuild{ - ObjectMeta: metav1.ObjectMeta{Name: name}, - Spec: mcfgv1.MachineOSBuildSpec{ - MachineConfig: mcfgv1.MachineConfigReference{Name: "rendered-worker-abc"}, - }, - } -} - -func testMOSC(name, pool string) *mcfgv1.MachineOSConfig { - return &mcfgv1.MachineOSConfig{ - ObjectMeta: metav1.ObjectMeta{Name: name}, - Spec: mcfgv1.MachineOSConfigSpec{ - MachineConfigPool: mcfgv1.MachineConfigPoolReference{Name: pool}, - }, - } -} - -func testMCP(name string) *mcfgv1.MachineConfigPool { - return &mcfgv1.MachineConfigPool{ - ObjectMeta: metav1.ObjectMeta{Name: name}, - } -} - -func testJob(name string) *batchv1.Job { - return &batchv1.Job{ - ObjectMeta: metav1.ObjectMeta{Name: name}, - } -} - -func TestBuildLifecycleEvents(t *testing.T) { - t.Parallel() - - mosb := testMOSB("test-build") - mosc := testMOSC("test-config", "worker") - - tests := []struct { - name string - record func(*OCLEventRecorder) - wantType string - wantReason string - }{ - {"BuildStarted", func(r *OCLEventRecorder) { r.RecordBuildStarted(mosb, mosc) }, "Normal", EventBuildStarted}, - {"BuildPreparing", func(r *OCLEventRecorder) { r.RecordBuildPreparing(mosb, "creating job") }, "Normal", EventBuildPreparing}, - {"BuildBuilding", func(r *OCLEventRecorder) { r.RecordBuildBuilding(mosb) }, "Normal", EventBuildBuilding}, - {"BuildCompleted", func(r *OCLEventRecorder) { r.RecordBuildCompleted(mosb, "quay.io/test@sha256:abc") }, "Normal", EventBuildCompleted}, - {"BuildFailed", func(r *OCLEventRecorder) { r.RecordBuildFailed(mosb) }, "Warning", EventBuildFailed}, - {"BuildInterrupted", func(r *OCLEventRecorder) { r.RecordBuildInterrupted(mosb, "job deleted") }, "Warning", EventBuildInterrupted}, - {"BuildDeleted", func(r *OCLEventRecorder) { r.RecordBuildDeleted(mosb, "removed") }, "Normal", EventBuildDeleted}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - recorder, fake := newTestOCLRecorder(1) - tt.record(recorder) - assertEvent(t, fake.Events, tt.wantType, tt.wantReason) - assertNoEvent(t, fake.Events) - }) - } -} - -func TestJobEvents(t *testing.T) { - t.Parallel() - - mosb := testMOSB("test-build") - job := testJob("test-job") - - tests := []struct { - name string - record func(*OCLEventRecorder) - wantType string - wantReason string - }{ - {"JobCreated", func(r *OCLEventRecorder) { r.RecordJobCreated(mosb, job) }, "Normal", EventJobCreated}, - {"JobStarted", func(r *OCLEventRecorder) { r.RecordJobStarted(mosb, job) }, "Normal", EventJobStarted}, - {"JobCompleted", func(r *OCLEventRecorder) { r.RecordJobCompleted(mosb, job) }, "Normal", EventJobCompleted}, - {"JobFailed", func(r *OCLEventRecorder) { r.RecordJobFailed(mosb, job) }, "Warning", EventJobFailed}, - {"JobDeleted", func(r *OCLEventRecorder) { r.RecordJobDeleted(mosb, job.Name) }, "Normal", EventJobDeleted}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - recorder, fake := newTestOCLRecorder(1) - tt.record(recorder) - assertEvent(t, fake.Events, tt.wantType, tt.wantReason) - assertNoEvent(t, fake.Events) - }) - } -} - -func TestConfigEvents(t *testing.T) { - t.Parallel() - - mosc := testMOSC("test-config", "worker") - - tests := []struct { - name string - record func(*OCLEventRecorder) - wantType string - wantReason string - }{ - {"ConfigReconciled", func(r *OCLEventRecorder) { r.RecordConfigReconciled(mosc) }, "Normal", EventConfigReconciled}, - {"ConfigReconcileFailed", func(r *OCLEventRecorder) { - r.RecordConfigReconcileFailed(mosc, fmt.Errorf("could not create MachineOSBuild")) - }, "Warning", EventConfigReconcileFailed}, - {"RebuildRequested", func(r *OCLEventRecorder) { r.RecordRebuildRequested(mosc, "annotation applied") }, "Normal", EventRebuildRequested}, - {"ConfigDeleted", func(r *OCLEventRecorder) { r.RecordConfigDeleted(mosc) }, "Normal", EventConfigDeleted}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - recorder, fake := newTestOCLRecorder(1) - tt.record(recorder) - assertEvent(t, fake.Events, tt.wantType, tt.wantReason) - assertNoEvent(t, fake.Events) - }) - } -} - -func TestPoolEvents(t *testing.T) { - t.Parallel() - - mcp := testMCP("worker") - - tests := []struct { - name string - record func(*OCLEventRecorder) - wantType string - wantReason string - }{ - {"PoolConfigChanged", func(r *OCLEventRecorder) { - r.RecordPoolConfigChanged(mcp, "rendered-worker-old", "rendered-worker-new") - }, "Normal", EventPoolConfigChanged}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - recorder, fake := newTestOCLRecorder(1) - tt.record(recorder) - assertEvent(t, fake.Events, tt.wantType, tt.wantReason) - assertNoEvent(t, fake.Events) - }) - } -} - -func TestDegradationEvents(t *testing.T) { - t.Parallel() - - mosc := testMOSC("test-config", "worker") - - tests := []struct { - name string - record func(*OCLEventRecorder) - wantType string - wantReason string - }{ - {"BuildDegraded", func(r *OCLEventRecorder) { r.RecordBuildDegraded(mosc) }, "Warning", EventBuildDegraded}, - {"BuildRecovered", func(r *OCLEventRecorder) { r.RecordBuildRecovered(mosc) }, "Normal", EventBuildRecovered}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - recorder, fake := newTestOCLRecorder(1) - tt.record(recorder) - assertEvent(t, fake.Events, tt.wantType, tt.wantReason) - assertNoEvent(t, fake.Events) - }) - } -} - -// TestBuildStartedMessageContent verifies the message includes the pool and rendered config name. -func TestBuildStartedMessageContent(t *testing.T) { - t.Parallel() - - mosb := testMOSB("test-build") - mosc := testMOSC("test-config", "worker") - - recorder, fake := newTestOCLRecorder(1) - recorder.RecordBuildStarted(mosb, mosc) - - event := <-fake.Events - if !strings.Contains(event, "worker") { - t.Errorf("expected event to contain pool name %q, got: %s", "worker", event) - } - if !strings.Contains(event, "rendered-worker-abc") { - t.Errorf("expected event to contain config name %q, got: %s", "rendered-worker-abc", event) - } -} diff --git a/pkg/controller/build/ocl_metrics.go b/pkg/controller/build/ocl_metrics.go deleted file mode 100644 index d0ac6595e2..0000000000 --- a/pkg/controller/build/ocl_metrics.go +++ /dev/null @@ -1,308 +0,0 @@ -package build - -import ( - "fmt" - "sync" - "time" - - ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" - "github.com/prometheus/client_golang/prometheus" -) - -// pushStartTimes stores the time each image push began, keyed by "pool/buildName". -// Used to compute push duration across separate AddJob and UpdateJob reconciler events. -var pushStartTimes sync.Map - -// OCL Build Metrics for tracking On-Cluster Layering processes -var ( - // oclBuildState tracks the current state of OCL builds per pool - oclBuildState = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Name: "ocl_build_state", - Help: "Current state of OCL build for a pool; gauge is 1 for the active state label, 0 otherwise", - }, []string{"pool", "state"}) - - // oclBuildDuration tracks how long OCL builds take to complete - oclBuildDuration = prometheus.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "ocl_build_duration_seconds", - Help: "Duration of OCL build processes in seconds", - Buckets: []float64{60, 180, 300, 600, 900, 1200, 1800, 2400, 3000, 3600}, // 1m to 1h - }, []string{"pool", "state"}) - - // oclBuildStartTime tracks when a build started - oclBuildStartTime = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Name: "ocl_build_start_timestamp_seconds", - Help: "Timestamp when OCL build started", - }, []string{"pool"}) - - // oclBuildEndTime tracks when a build completed/failed - oclBuildEndTime = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Name: "ocl_build_end_timestamp_seconds", - Help: "Timestamp when OCL build completed or failed", - }, []string{"pool", "state"}) - - // oclBuildTotal counts total number of builds by state - oclBuildTotal = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Name: "ocl_build_total", - Help: "Total number of OCL builds by final state", - }, []string{"pool", "state"}) - - // oclBuildJobState tracks the state of build jobs - oclBuildJobState = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Name: "ocl_build_job_state", - Help: "State of OCL build job; gauge is 1 for the active state label, 0 otherwise", - }, []string{"pool", "state"}) - - // oclConfigChangeTotal counts config changes triggering builds - oclConfigChangeTotal = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Name: "ocl_config_change_total", - Help: "Total number of config changes triggering OCL builds", - }, []string{"pool"}) - - // oclBuildRetries tracks build retry attempts - oclBuildRetries = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Name: "ocl_build_retries_total", - Help: "Total number of OCL build retry attempts", - }, []string{"pool"}) - - // oclLayeredNodesCount tracks number of nodes using layered images - oclLayeredNodesCount = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Name: "ocl_layered_nodes_count", - Help: "Number of nodes currently using OCL layered images", - }, []string{"pool"}) - - // oclImagePushState tracks the state of image push operations - oclImagePushState = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Name: "ocl_image_push_state", - Help: "State of OCL image push operations; gauge is 1 for the active state label, 0 otherwise", - }, []string{"pool", "state"}) - - // oclImagePushTotal counts total image push attempts by final state - oclImagePushTotal = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Name: "ocl_image_push_total", - Help: "Total number of OCL image push operations by final state", - }, []string{"pool", "state"}) - - // oclRolloutUpdatedNodes tracks how many nodes have adopted the current layered image - oclRolloutUpdatedNodes = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Name: "ocl_rollout_updated_nodes", - Help: "Number of nodes in an OCL pool that have adopted the current layered image", - }, []string{"pool"}) - - // oclRolloutTotalNodes tracks total node count in an OCL pool - oclRolloutTotalNodes = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Name: "ocl_rollout_total_nodes", - Help: "Total number of nodes in an OCL pool", - }, []string{"pool"}) - - // oclBuildQueueDuration tracks time between build creation and job going active - oclBuildQueueDuration = prometheus.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "ocl_build_queue_duration_seconds", - Help: "Time in seconds between OCL build creation and the build job becoming active", - Buckets: []float64{5, 15, 30, 60, 120, 300, 600}, - }, []string{"pool"}) - - // oclImagePushDuration tracks how long image push operations take - oclImagePushDuration = prometheus.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "ocl_image_push_duration_seconds", - Help: "Duration of OCL image push operations in seconds", - Buckets: []float64{10, 30, 60, 120, 180, 300, 600}, - }, []string{"pool", "state"}) - - // oclActiveBuilds tracks the number of builds currently in progress per pool - oclActiveBuilds = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Name: "ocl_active_builds", - Help: "Number of OCL builds currently in progress per pool", - }, []string{"pool"}) - - // oclMOSCCount is the number of MachineOSConfig objects in the cluster, used to determine OCL adoption - oclMOSCCount = prometheus.NewGauge( - prometheus.GaugeOpts{ - Name: "mco_mosc_count", - Help: "number of MachineOSConfig objects in the cluster; non-zero value indicates on-cluster layering is in use", - }) -) - -// Build state constants for consistent labeling -const ( - StateNone = "none" - StatePending = "pending" - StateBuilding = "building" - StateSucceeded = "succeeded" - StateFailed = "failed" - StateInterrupted = "interrupted" - StatePushing = "pushing" -) - -// RegisterOCLMetrics registers all OCL-related Prometheus metrics -func RegisterOCLMetrics() error { - err := ctrlcommon.RegisterMetrics([]prometheus.Collector{ - oclBuildState, - oclBuildDuration, - oclBuildStartTime, - oclBuildEndTime, - oclBuildTotal, - oclBuildJobState, - oclConfigChangeTotal, - oclBuildRetries, - oclLayeredNodesCount, - oclImagePushState, - oclImagePushTotal, - oclRolloutUpdatedNodes, - oclRolloutTotalNodes, - oclBuildQueueDuration, - oclImagePushDuration, - oclActiveBuilds, - oclMOSCCount, - }) - - if err != nil { - return fmt.Errorf("could not register OCL metrics: %w", err) - } - - return nil -} - -// RecordBuildStarted records when a build starts -func RecordBuildStarted(pool string) { - now := float64(time.Now().Unix()) - - // Clear previous states for this pool - oclBuildState.DeletePartialMatch(prometheus.Labels{"pool": pool}) - - // Set new state - oclBuildState.WithLabelValues(pool, StatePending).Set(1) - oclBuildStartTime.WithLabelValues(pool).Set(now) - oclActiveBuilds.WithLabelValues(pool).Inc() -} - -// RecordBuildBuilding records when a build transitions to building state -func RecordBuildBuilding(pool string) { - // Clear pending state - oclBuildState.DeletePartialMatch(prometheus.Labels{"pool": pool}) - - // Set building state - oclBuildState.WithLabelValues(pool, StateBuilding).Set(1) -} - -// RecordBuildCompleted records when a build completes successfully -func RecordBuildCompleted(pool string, startTime time.Time) { - now := time.Now() - duration := now.Sub(startTime).Seconds() - - // Clear previous states - oclBuildState.DeletePartialMatch(prometheus.Labels{"pool": pool}) - pushStartTimes.Delete(pool) - - // Set succeeded state - oclBuildState.WithLabelValues(pool, StateSucceeded).Set(1) - oclBuildEndTime.WithLabelValues(pool, StateSucceeded).Set(float64(now.Unix())) - oclBuildDuration.WithLabelValues(pool, StateSucceeded).Observe(duration) - oclBuildTotal.WithLabelValues(pool, StateSucceeded).Inc() - oclActiveBuilds.WithLabelValues(pool).Dec() -} - -// RecordBuildFailed records when a build fails -func RecordBuildFailed(pool string, startTime time.Time) { - now := time.Now() - duration := now.Sub(startTime).Seconds() - - // Clear previous states - oclBuildState.DeletePartialMatch(prometheus.Labels{"pool": pool}) - pushStartTimes.Delete(pool) - - // Set failed state - oclBuildState.WithLabelValues(pool, StateFailed).Set(1) - oclBuildEndTime.WithLabelValues(pool, StateFailed).Set(float64(now.Unix())) - oclBuildDuration.WithLabelValues(pool, StateFailed).Observe(duration) - oclBuildTotal.WithLabelValues(pool, StateFailed).Inc() - oclActiveBuilds.WithLabelValues(pool).Dec() -} - -// RecordBuildInterrupted records when a build is interrupted -func RecordBuildInterrupted(pool string) { - // Clear previous states - oclBuildState.DeletePartialMatch(prometheus.Labels{"pool": pool}) - pushStartTimes.Delete(pool) - - // Set interrupted state - oclBuildState.WithLabelValues(pool, StateInterrupted).Set(1) - oclBuildEndTime.WithLabelValues(pool, StateInterrupted).Set(float64(time.Now().Unix())) - oclBuildTotal.WithLabelValues(pool, StateInterrupted).Inc() - oclActiveBuilds.WithLabelValues(pool).Dec() -} - -// RecordBuildJobState records the state of a build job -func RecordBuildJobState(pool, state string) { - oclBuildJobState.DeletePartialMatch(prometheus.Labels{"pool": pool}) - oclBuildJobState.WithLabelValues(pool, state).Set(1) -} - -// RecordConfigChange records when a config change triggers a build -func RecordConfigChange(pool string) { - oclConfigChangeTotal.WithLabelValues(pool).Inc() -} - -// RecordBuildRetry records a build retry attempt -func RecordBuildRetry(pool string) { - oclBuildRetries.WithLabelValues(pool).Inc() -} - -// UpdateLayeredNodesCount updates the count of nodes using layered images -func UpdateLayeredNodesCount(pool string, count int) { - oclLayeredNodesCount.WithLabelValues(pool).Set(float64(count)) -} - -// RecordImagePushStarted records when a build job becomes active (image push begins). -func RecordImagePushStarted(pool string) { - oclImagePushState.DeletePartialMatch(prometheus.Labels{"pool": pool}) - oclImagePushState.WithLabelValues(pool, StatePushing).Set(1) - pushStartTimes.Store(pool, time.Now()) -} - -// RecordImagePushCompleted records a successful image push. -func RecordImagePushCompleted(pool string) { - oclImagePushState.DeletePartialMatch(prometheus.Labels{"pool": pool}) - oclImagePushState.WithLabelValues(pool, StateSucceeded).Set(1) - oclImagePushTotal.WithLabelValues(pool, StateSucceeded).Inc() - if start, ok := pushStartTimes.LoadAndDelete(pool); ok { - oclImagePushDuration.WithLabelValues(pool, StateSucceeded).Observe(time.Since(start.(time.Time)).Seconds()) - } -} - -// RecordImagePushFailed records a failed image push. -func RecordImagePushFailed(pool string) { - oclImagePushState.DeletePartialMatch(prometheus.Labels{"pool": pool}) - oclImagePushState.WithLabelValues(pool, StateFailed).Set(1) - oclImagePushTotal.WithLabelValues(pool, StateFailed).Inc() - if start, ok := pushStartTimes.LoadAndDelete(pool); ok { - oclImagePushDuration.WithLabelValues(pool, StateFailed).Observe(time.Since(start.(time.Time)).Seconds()) - } -} - -// RecordBuildQueueDuration records how long a build waited before its job went active. -// queuedAt should be the MachineOSBuild's CreationTimestamp. -func RecordBuildQueueDuration(pool string, queuedAt time.Time) { - oclBuildQueueDuration.WithLabelValues(pool).Observe(time.Since(queuedAt).Seconds()) -} - -// UpdateOCLRolloutCounts updates the OCL rollout node counts from MachineConfigPool status. -func UpdateOCLRolloutCounts(pool string, updatedNodes, totalNodes int32) { - oclRolloutUpdatedNodes.WithLabelValues(pool).Set(float64(updatedNodes)) - oclRolloutTotalNodes.WithLabelValues(pool).Set(float64(totalNodes)) -} diff --git a/pkg/controller/build/ocl_metrics_test.go b/pkg/controller/build/ocl_metrics_test.go deleted file mode 100644 index 70c28d95d7..0000000000 --- a/pkg/controller/build/ocl_metrics_test.go +++ /dev/null @@ -1,223 +0,0 @@ -package build - -import ( - "testing" - "time" - - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/testutil" -) - -// resetMetric deletes all label combinations for the given GaugeVec so tests -// don't bleed into each other. -func resetGaugeVec(g *prometheus.GaugeVec, labels prometheus.Labels) { - g.DeletePartialMatch(labels) -} - -func TestRecordImagePushStarted(t *testing.T) { - t.Parallel() - - resetGaugeVec(oclImagePushState, prometheus.Labels{"pool": "worker"}) - - RecordImagePushStarted("worker") - - v := testutil.ToFloat64(oclImagePushState.WithLabelValues("worker", StatePushing)) - if v != 1 { - t.Errorf("expected ocl_image_push_state{state=%q} = 1, got %v", StatePushing, v) - } -} - -func TestRecordImagePushCompleted(t *testing.T) { - t.Parallel() - - resetGaugeVec(oclImagePushState, prometheus.Labels{"pool": "worker2"}) - - RecordImagePushStarted("worker2") - RecordImagePushCompleted("worker2") - - gauge := testutil.ToFloat64(oclImagePushState.WithLabelValues("worker2", StateSucceeded)) - if gauge != 1 { - t.Errorf("expected ocl_image_push_state{state=%q} = 1, got %v", StateSucceeded, gauge) - } - - // pushing state should be cleared - pushing := testutil.ToFloat64(oclImagePushState.WithLabelValues("worker2", StatePushing)) - if pushing != 0 { - t.Errorf("expected ocl_image_push_state{state=%q} = 0 after completion, got %v", StatePushing, pushing) - } -} - -func TestRecordImagePushFailed(t *testing.T) { - t.Parallel() - - resetGaugeVec(oclImagePushState, prometheus.Labels{"pool": "worker3"}) - - RecordImagePushStarted("worker3") - RecordImagePushFailed("worker3") - - gauge := testutil.ToFloat64(oclImagePushState.WithLabelValues("worker3", StateFailed)) - if gauge != 1 { - t.Errorf("expected ocl_image_push_state{state=%q} = 1, got %v", StateFailed, gauge) - } - - // pushing state should be cleared - pushing := testutil.ToFloat64(oclImagePushState.WithLabelValues("worker3", StatePushing)) - if pushing != 0 { - t.Errorf("expected ocl_image_push_state{state=%q} = 0 after failure, got %v", StatePushing, pushing) - } -} - -func TestImagePushTotalCounters(t *testing.T) { - t.Parallel() - - // Use unique pool name to avoid counter bleed from other tests - pool := "counter-test-pool" - - before := testutil.ToFloat64(oclImagePushTotal.WithLabelValues(pool, StateSucceeded)) - RecordImagePushCompleted(pool) - after := testutil.ToFloat64(oclImagePushTotal.WithLabelValues(pool, StateSucceeded)) - if after-before != 1 { - t.Errorf("expected ocl_image_push_total{state=%q} to increment by 1, got %v -> %v", StateSucceeded, before, after) - } - - before = testutil.ToFloat64(oclImagePushTotal.WithLabelValues(pool, StateFailed)) - RecordImagePushFailed(pool) - after = testutil.ToFloat64(oclImagePushTotal.WithLabelValues(pool, StateFailed)) - if after-before != 1 { - t.Errorf("expected ocl_image_push_total{state=%q} to increment by 1, got %v -> %v", StateFailed, before, after) - } -} - -func TestUpdateOCLRolloutCounts(t *testing.T) { - t.Parallel() - - pool := "rollout-pool" - - UpdateOCLRolloutCounts(pool, 3, 5) - - updated := testutil.ToFloat64(oclRolloutUpdatedNodes.WithLabelValues(pool)) - if updated != 3 { - t.Errorf("expected ocl_rollout_updated_nodes = 3, got %v", updated) - } - - total := testutil.ToFloat64(oclRolloutTotalNodes.WithLabelValues(pool)) - if total != 5 { - t.Errorf("expected ocl_rollout_total_nodes = 5, got %v", total) - } -} - -func TestUpdateOCLRolloutCountsUpdates(t *testing.T) { - t.Parallel() - - pool := "rollout-pool2" - - UpdateOCLRolloutCounts(pool, 2, 10) - UpdateOCLRolloutCounts(pool, 8, 10) - - updated := testutil.ToFloat64(oclRolloutUpdatedNodes.WithLabelValues(pool)) - if updated != 8 { - t.Errorf("expected ocl_rollout_updated_nodes to reflect latest value 8, got %v", updated) - } -} - -func TestActiveBuildsGauge(t *testing.T) { - t.Parallel() - - pool := "active-builds-pool" - oclActiveBuilds.WithLabelValues(pool).Set(0) - - RecordBuildStarted(pool) - if v := testutil.ToFloat64(oclActiveBuilds.WithLabelValues(pool)); v != 1 { - t.Errorf("expected ocl_active_builds = 1 after start, got %v", v) - } - - RecordBuildStarted(pool) - if v := testutil.ToFloat64(oclActiveBuilds.WithLabelValues(pool)); v != 2 { - t.Errorf("expected ocl_active_builds = 2 after second start, got %v", v) - } - - RecordBuildCompleted(pool, time.Now()) - if v := testutil.ToFloat64(oclActiveBuilds.WithLabelValues(pool)); v != 1 { - t.Errorf("expected ocl_active_builds = 1 after completion, got %v", v) - } - - RecordBuildFailed(pool, time.Now()) - if v := testutil.ToFloat64(oclActiveBuilds.WithLabelValues(pool)); v != 0 { - t.Errorf("expected ocl_active_builds = 0 after failure, got %v", v) - } -} - -func TestActiveBuildsDecrementsOnInterrupted(t *testing.T) { - t.Parallel() - - pool := "interrupted-pool" - oclActiveBuilds.WithLabelValues(pool).Set(0) - - RecordBuildStarted(pool) - RecordBuildInterrupted(pool) - - if v := testutil.ToFloat64(oclActiveBuilds.WithLabelValues(pool)); v != 0 { - t.Errorf("expected ocl_active_builds = 0 after interruption, got %v", v) - } -} - -func TestBuildQueueDuration(t *testing.T) { - t.Parallel() - - pool := "queue-pool" - queuedAt := time.Now().Add(-30 * time.Second) - - before := testutil.CollectAndCount(oclBuildQueueDuration) - RecordBuildQueueDuration(pool, queuedAt) - after := testutil.CollectAndCount(oclBuildQueueDuration) - - if after <= before { - t.Errorf("expected ocl_build_queue_duration_seconds to have more observations after recording, got count %v -> %v", before, after) - } -} - -func TestImagePushDurationRecorded(t *testing.T) { - t.Parallel() - - pool := "push-duration-pool" - - before := testutil.CollectAndCount(oclImagePushDuration) - RecordImagePushStarted(pool) - time.Sleep(5 * time.Millisecond) - RecordImagePushCompleted(pool) - after := testutil.CollectAndCount(oclImagePushDuration) - - if after <= before { - t.Errorf("expected ocl_image_push_duration_seconds to have more observations after push completed, got count %v -> %v", before, after) - } -} - -func TestImagePushDurationOnFailure(t *testing.T) { - t.Parallel() - - pool := "push-duration-fail-pool" - - before := testutil.CollectAndCount(oclImagePushDuration) - RecordImagePushStarted(pool) - time.Sleep(5 * time.Millisecond) - RecordImagePushFailed(pool) - after := testutil.CollectAndCount(oclImagePushDuration) - - if after <= before { - t.Errorf("expected ocl_image_push_duration_seconds to have more observations after push failed, got count %v -> %v", before, after) - } -} - -func TestOCLMOSCCount(t *testing.T) { - t.Parallel() - - oclMOSCCount.Set(0) - if v := testutil.ToFloat64(oclMOSCCount); v != 0 { - t.Errorf("expected mco_mosc_count = 0, got %v", v) - } - - oclMOSCCount.Set(3) - if v := testutil.ToFloat64(oclMOSCCount); v != 3 { - t.Errorf("expected mco_mosc_count = 3, got %v", v) - } -} diff --git a/pkg/controller/build/osbuildcontroller.go b/pkg/controller/build/osbuildcontroller.go index 893768469e..073f6b128c 100644 --- a/pkg/controller/build/osbuildcontroller.go +++ b/pkg/controller/build/osbuildcontroller.go @@ -149,7 +149,7 @@ func newOSBuildController( UpdateFunc: ctrl.updateMachineConfigPool, }) - ctrl.buildReconciler = newBuildReconciler(mcfgclient, kubeclient, imageclient, routeclient, ctrl.listers, imagepruner, ctrl.eventRecorder) + ctrl.buildReconciler = newBuildReconciler(mcfgclient, kubeclient, imageclient, routeclient, ctrl.listers, imagepruner) ctrl.shutdownDelayHandler = newShutdownDelayHandler(ctrl.listers) ctrl.shutdownChan = make(chan struct{}) diff --git a/pkg/controller/build/reconciler.go b/pkg/controller/build/reconciler.go index 09daba4436..34c2b8cdbd 100644 --- a/pkg/controller/build/reconciler.go +++ b/pkg/controller/build/reconciler.go @@ -27,7 +27,6 @@ import ( k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/labels" clientset "k8s.io/client-go/kubernetes" - "k8s.io/client-go/tools/record" "k8s.io/client-go/util/retry" "k8s.io/klog/v2" @@ -63,30 +62,28 @@ type reconciler interface { // is to respond to incoming events in a specific way. By doing this, the // reconciliation process has a clear entrypoint for each incoming event. type buildReconciler struct { - mcfgclient mcfgclientset.Interface - kubeclient clientset.Interface - imageclient imagev1clientset.Interface - routeclient routeclientset.Interface - imagepruner imagepruner.ImagePruner - eventRecorder *OCLEventRecorder + mcfgclient mcfgclientset.Interface + kubeclient clientset.Interface + imageclient imagev1clientset.Interface + routeclient routeclientset.Interface + imagepruner imagepruner.ImagePruner *listers } // Instantiates a new reconciler instance. This returns an interface to // disallow access to its private methods. -func newBuildReconciler(mcfgclient mcfgclientset.Interface, kubeclient clientset.Interface, imageclient imagev1clientset.Interface, routeclient routeclientset.Interface, l *listers, imagepruner imagepruner.ImagePruner, eventRecorder record.EventRecorder) reconciler { - return newBuildReconcilerAsStruct(mcfgclient, kubeclient, imageclient, routeclient, l, imagepruner, eventRecorder) +func newBuildReconciler(mcfgclient mcfgclientset.Interface, kubeclient clientset.Interface, imageclient imagev1clientset.Interface, routeclient routeclientset.Interface, l *listers, imagepruner imagepruner.ImagePruner) reconciler { + return newBuildReconcilerAsStruct(mcfgclient, kubeclient, imageclient, routeclient, l, imagepruner) } -func newBuildReconcilerAsStruct(mcfgclient mcfgclientset.Interface, kubeclient clientset.Interface, imageclient imagev1clientset.Interface, routeclient routeclientset.Interface, l *listers, imagepruner imagepruner.ImagePruner, eventRecorder record.EventRecorder) *buildReconciler { +func newBuildReconcilerAsStruct(mcfgclient mcfgclientset.Interface, kubeclient clientset.Interface, imageclient imagev1clientset.Interface, routeclient routeclientset.Interface, l *listers, imagepruner imagepruner.ImagePruner) *buildReconciler { return &buildReconciler{ - mcfgclient: mcfgclient, - kubeclient: kubeclient, - imageclient: imageclient, - routeclient: routeclient, - imagepruner: imagepruner, - eventRecorder: NewOCLEventRecorder(eventRecorder), - listers: l, + mcfgclient: mcfgclient, + kubeclient: kubeclient, + imageclient: imageclient, + routeclient: routeclient, + imagepruner: imagepruner, + listers: l, } } @@ -123,12 +120,7 @@ func (b *buildReconciler) updateMachineOSConfig(ctx context.Context, old, cur *m // Whenever the MachineOSConfig spec has changed, create a new MachineOSBuild. if !equality.Semantic.DeepEqual(old.Spec, cur.Spec) { klog.Infof("Detected MachineOSConfig change for %s", cur.Name) - if err := b.createNewMachineOSBuildOrReuseExisting(ctx, cur, false); err != nil { - b.eventRecorder.RecordConfigReconcileFailed(cur, err) - return err - } - b.eventRecorder.RecordConfigReconciled(cur) - return nil + return b.createNewMachineOSBuildOrReuseExisting(ctx, cur, false) } return b.syncMachineOSConfigs(ctx) @@ -140,8 +132,6 @@ func (b *buildReconciler) updateMachineOSConfig(ctx context.Context, old, cur *m func (b *buildReconciler) rebuildMachineOSConfig(ctx context.Context, mosc *mcfgv1.MachineOSConfig) error { klog.Infof("MachineOSConfig %q has rebuild annotation (%q)", mosc.Name, constants.RebuildMachineOSConfigAnnotationKey) - b.eventRecorder.RecordRebuildRequested(mosc, "rebuild annotation applied") - if !hasCurrentBuildAnnotation(mosc) { klog.Infof("MachineOSConfig %q does not have current build annotation (%q) set, skipping rebuild", mosc.Name, constants.CurrentMachineOSBuildAnnotationKey) return nil @@ -220,7 +210,7 @@ func (b *buildReconciler) deleteMachineOSConfig(ctx context.Context, mosc *mcfgv return fmt.Errorf("could not delete MachineOSBuild %s for MachineOSConfig %s: %w", mosb.Name, mosc.Name, err) } } - b.eventRecorder.RecordConfigDeleted(mosc) + return nil } @@ -230,18 +220,6 @@ func (b *buildReconciler) AddJob(ctx context.Context, job *batchv1.Job) error { return b.timeObjectOperation(job, addingVerb, func() error { klog.Infof("Adding build job %q", job.Name) - mosb, err := b.getMachineOSBuildForJob(job) - if err == nil && mosb != nil { - b.eventRecorder.RecordJobCreated(mosb, job) - mosc, err := utils.GetMachineOSConfigForMachineOSBuild(mosb, b.utilListers()) - if err == nil { - poolName := mosc.Spec.MachineConfigPool.Name - RecordBuildJobState(poolName, "active") - RecordImagePushStarted(poolName) - RecordBuildQueueDuration(poolName, mosb.CreationTimestamp.Time) - } - } - if err := b.updateMachineOSBuildWithStatus(ctx, job); err != nil { return fmt.Errorf("could not update job status for %q: %w", job.Name, err) } @@ -253,41 +231,6 @@ func (b *buildReconciler) AddJob(ctx context.Context, job *batchv1.Job) error { // Executes whenever a build Job is updated func (b *buildReconciler) UpdateJob(ctx context.Context, oldJob, curJob *batchv1.Job) error { return b.timeObjectOperation(curJob, updatingVerb, func() error { - mosb, err := b.getMachineOSBuildForJob(curJob) - if err == nil && mosb != nil { - if curJob.Status.Succeeded > 0 && (oldJob.Status.Succeeded == 0) { - b.eventRecorder.RecordJobCompleted(mosb, curJob) - } - - if curJob.Status.Failed > 0 && (oldJob.Status.Failed == 0) { - b.eventRecorder.RecordJobFailed(mosb, curJob) - } - - if curJob.Status.Active > 0 && (oldJob.Status.Active == 0) { - b.eventRecorder.RecordJobStarted(mosb, curJob) - b.eventRecorder.RecordBuildBuilding(mosb) - } - - mosc, err := utils.GetMachineOSConfigForMachineOSBuild(mosb, b.utilListers()) - if err == nil { - poolName := mosc.Spec.MachineConfigPool.Name - - if curJob.Status.Succeeded > 0 && (oldJob.Status.Succeeded == 0) { - RecordBuildJobState(poolName, StateSucceeded) - RecordImagePushCompleted(poolName) - } - - if curJob.Status.Failed > 0 && (oldJob.Status.Failed == 0) { - RecordBuildJobState(poolName, StateFailed) - RecordImagePushFailed(poolName) - } - - if curJob.Status.Failed > oldJob.Status.Failed && curJob.Status.Failed <= constants.JobMaxRetries { - RecordBuildRetry(poolName) - } - } - } - return b.updateMachineOSBuildWithStatusIfNeeded(ctx, oldJob, curJob) }) } @@ -295,18 +238,10 @@ func (b *buildReconciler) UpdateJob(ctx context.Context, oldJob, curJob *batchv1 // Executes whenever a build Job is deleted func (b *buildReconciler) DeleteJob(ctx context.Context, job *batchv1.Job) error { return b.timeObjectOperation(job, deletingVerb, func() error { - mosb, err := b.getMachineOSBuildForJob(job) - if err == nil && mosb != nil { - b.eventRecorder.RecordJobDeleted(mosb, job.Name) - if !ctrlcommon.NewMachineOSBuildState(mosb).IsBuildSuccess() { - b.eventRecorder.RecordBuildInterrupted(mosb, "build job was deleted") - } - } + // Set the DeletionTimestamp so that we can set the build status to interrupted + job.SetDeletionTimestamp(&metav1.Time{Time: time.Now()}) - jobCopy := job.DeepCopy() - jobCopy.SetDeletionTimestamp(&metav1.Time{Time: time.Now()}) - - err = b.updateMachineOSBuildWithStatus(ctx, jobCopy) + err := b.updateMachineOSBuildWithStatus(ctx, job) if err != nil { return err } @@ -353,31 +288,16 @@ func (b *buildReconciler) updateMachineOSBuild(ctx context.Context, old, current return nil } - poolName := mosc.Spec.MachineConfigPool.Name - if !oldState.IsBuildFailure() && curState.IsBuildFailure() { klog.Infof("MachineOSBuild %s failed, leaving ephemeral objects in place for inspection", current.Name) - if old.CreationTimestamp.Time.IsZero() { - RecordBuildFailed(poolName, time.Now()) - } else { - RecordBuildFailed(poolName, old.CreationTimestamp.Time) - } - mcp, err := b.machineConfigPoolLister.Get(mosc.Spec.MachineConfigPool.Name) if err != nil { return fmt.Errorf("could not get MachineConfigPool from MachineOSConfig %q: %w", mosc.Name, err) } // Always update ImageBuildDegraded condition based on current active build status - if err := b.updateImageBuildDegradedCondition(ctx, mcp, mosc); err != nil { - return err - } - - b.eventRecorder.RecordBuildFailed(current) - b.eventRecorder.RecordBuildDegraded(mosc) - - return nil + return b.updateImageBuildDegradedCondition(ctx, mcp, mosc) } // If the build was successful, clean up the build objects and propagate the @@ -386,29 +306,11 @@ func (b *buildReconciler) updateMachineOSBuild(ctx context.Context, old, current if !oldState.IsBuildSuccess() && curState.IsBuildSuccess() { klog.Infof("MachineOSBuild %s succeeded, cleaning up all ephemeral objects used for the build", current.Name) - b.eventRecorder.RecordBuildCompleted(current, string(current.Status.DigestedImagePushSpec)) - - if old.CreationTimestamp.Time.IsZero() { - RecordBuildCompleted(poolName, time.Now()) - } else { - RecordBuildCompleted(poolName, old.CreationTimestamp.Time) - } - mcp, err := b.machineConfigPoolLister.Get(mosc.Spec.MachineConfigPool.Name) if err != nil { return fmt.Errorf("could not get MachineConfigPool from MachineOSConfig %q: %w", mosc.Name, err) } - // Use a live Get to check the actual degraded state — the lister may already - // reflect the cleared condition from initializeBuildDegradedCondition running - // when the new build started, causing a false negative. - liveMCP, err := b.mcfgclient.MachineconfigurationV1().MachineConfigPools().Get(ctx, mcp.Name, metav1.GetOptions{}) - if err != nil { - klog.Warningf("Could not live-fetch MachineConfigPool %s to check recovery state: %v", mcp.Name, err) - } else if apihelpers.IsMachineConfigPoolConditionTrue(liveMCP.Status.Conditions, mcfgv1.MachineConfigPoolImageBuildDegraded) { - b.eventRecorder.RecordBuildRecovered(mosc) - } - // Update BuildDegraded condition based on current active build status if err := b.updateImageBuildDegradedCondition(ctx, mcp, mosc); err != nil { klog.Errorf("Failed to update ImageBuildDegraded condition for pool %s: %v", mcp.Name, err) @@ -424,14 +326,6 @@ func (b *buildReconciler) updateMachineOSBuild(ctx context.Context, old, current } } - if !oldState.IsBuilding() && curState.IsBuilding() { - RecordBuildBuilding(poolName) - } - - if !oldState.IsBuildInterrupted() && curState.IsBuildInterrupted() { - RecordBuildInterrupted(poolName) - } - return nil } @@ -537,17 +431,11 @@ func (b *buildReconciler) UpdateMachineConfigPool(ctx context.Context, oldMCP, c func (b *buildReconciler) updateMachineConfigPool(ctx context.Context, oldMCP, curMCP *mcfgv1.MachineConfigPool) error { if oldMCP.Spec.Configuration.Name != curMCP.Spec.Configuration.Name { klog.Infof("Rendered config for pool %s changed from %s to %s", curMCP.Name, oldMCP.Spec.Configuration.Name, curMCP.Spec.Configuration.Name) - b.eventRecorder.RecordPoolConfigChanged(curMCP, oldMCP.Spec.Configuration.Name, curMCP.Spec.Configuration.Name) - RecordConfigChange(curMCP.Name) if err := b.reconcilePoolChange(ctx, curMCP); err != nil { return fmt.Errorf("could not create or reuse existing MachineOSBuild for MachineConfigPool %q change: %w", curMCP.Name, err) } } - if _, err := utils.GetMachineOSConfigForMachineConfigPool(curMCP, b.utilListers()); err == nil { - UpdateOCLRolloutCounts(curMCP.Name, curMCP.Status.UpdatedMachineCount, curMCP.Status.MachineCount) - } - return b.syncAll(ctx) } @@ -563,17 +451,11 @@ func (b *buildReconciler) startBuild(ctx context.Context, mosb *mcfgv1.MachineOS return err } - poolName := mosc.Spec.MachineConfigPool.Name - // If there are any other in-progress builds for this MachineOSConfig, stop them first. if err := b.deleteOtherBuildsForMachineOSConfig(ctx, mosb, mosc); err != nil { return fmt.Errorf("could not delete other non-terminal MachineOSBuilds for MachineOSConfig %s: %w", mosc.Name, err) } - b.eventRecorder.RecordBuildStarted(mosb, mosc) - b.eventRecorder.RecordBuildPreparing(mosb, fmt.Sprintf("creating build job for pool %q", mosc.Spec.MachineConfigPool.Name)) - RecordBuildStarted(poolName) - // Next, create our new MachineOSBuild. if err := imagebuilder.NewJobImageBuilder(b.kubeclient, b.mcfgclient, mosb, mosc).Start(ctx); err != nil { var validationErr *buildrequest.ContainerfileValidationError @@ -590,8 +472,6 @@ func (b *buildReconciler) startBuild(ctx context.Context, mosb *mcfgv1.MachineOS return fmt.Errorf("imagebuilder could not start build for MachineOSBuild %q: %w", mosb.Name, err) } - RecordBuildStarted(poolName) - klog.Infof("Started new build %s for MachineOSBuild", utils.GetBuildJobName(mosb)) if err := b.updateMachineOSConfigStatus(ctx, mosc, mosb); err != nil { @@ -622,16 +502,6 @@ func (b *buildReconciler) getMachineOSConfigForUpdate(mosc *mcfgv1.MachineOSConf return out.DeepCopy(), nil } -// Retrieves the MachineOSBuild associated with a Job based on labels -func (b *buildReconciler) getMachineOSBuildForJob(job *batchv1.Job) (*mcfgv1.MachineOSBuild, error) { - if !metav1.HasLabel(job.ObjectMeta, constants.MachineOSBuildNameLabelKey) { - return nil, fmt.Errorf("job %q does not have MachineOSBuild label", job.Name) - } - - mosbName := job.Labels[constants.MachineOSBuildNameLabelKey] - return b.machineOSBuildLister.Get(mosbName) -} - // Retrieves a deep-copy of the MachineOSBuild from the lister so that the cache is not mutated during the update. func (b *buildReconciler) getMachineOSBuildForUpdate(mosb *mcfgv1.MachineOSBuild) (*mcfgv1.MachineOSBuild, error) { out, err := b.machineOSBuildLister.Get(mosb.Name) @@ -1064,13 +934,11 @@ func (b *buildReconciler) deleteMachineOSBuild(ctx context.Context, mosb *mcfgv1 err = b.mcfgclient.MachineconfigurationV1().MachineOSBuilds().Delete(ctx, mosb.Name, metav1.DeleteOptions{}) if err == nil { klog.Infof("Deleted MachineOSBuild %s for MachineOSConfig %s", mosb.Name, moscName) - b.eventRecorder.RecordBuildDeleted(mosb, "build resources removed") return nil } if k8serrors.IsNotFound(err) { klog.Infof("MachineOSBuild %s was not found for MachineOSConfig %s", mosb.Name, moscName) - b.eventRecorder.RecordBuildDeleted(mosb, "build resources removed") return nil } @@ -1387,8 +1255,6 @@ func (b *buildReconciler) syncMachineOSConfigs(ctx context.Context) error { return err } - oclMOSCCount.Set(float64(len(moscs))) - for _, mosc := range moscs { if err := b.syncMachineOSConfig(ctx, mosc); err != nil { return fmt.Errorf("could not sync MachineOSConfig %q: %w", mosc.Name, err) diff --git a/pkg/controller/common/controller_context.go b/pkg/controller/common/controller_context.go index 410d1b743a..99e6733610 100644 --- a/pkg/controller/common/controller_context.go +++ b/pkg/controller/common/controller_context.go @@ -8,9 +8,7 @@ import ( configinformers "github.com/openshift/client-go/config/informers/externalversions" imageinformers "github.com/openshift/client-go/image/informers/externalversions" machineinformersv1beta1 "github.com/openshift/client-go/machine/informers/externalversions" - mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned" mcfginformers "github.com/openshift/client-go/machineconfiguration/informers/externalversions" - mcfginformersv1 "github.com/openshift/client-go/machineconfiguration/informers/externalversions/machineconfiguration/v1" operatorinformers "github.com/openshift/client-go/operator/informers/externalversions" routeinformers "github.com/openshift/client-go/route/informers/externalversions" "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" @@ -61,10 +59,6 @@ type ControllerContext struct { OpenShiftConfigKubeNamespacedInformerFactory informers.SharedInformerFactory OpenShiftConfigManagedKubeNamespacedInformerFactory informers.SharedInformerFactory OpenShiftKubeAPIServerKubeNamespacedInformerFactory informers.SharedInformerFactory - // Scoped informer factory for MCO-specific pods only (namespace + label filtered) - // This dramatically reduces memory by only caching machine-config-operator pods - // instead of all pods cluster-wide - MCOPodInformerFactory informers.SharedInformerFactory APIExtInformerFactory apiextinformers.SharedInformerFactory ConfigInformerFactory configinformers.SharedInformerFactory OperatorInformerFactory operatorinformers.SharedInformerFactory @@ -110,17 +104,6 @@ func CreateControllerContext(ctx context.Context, cb *clients.Builder) *Controll ) // this is needed to listen for changes in MAO user data secrets to re-apply the ones we define in the MCO (since we manage them) kubeMAOSharedInformer := informers.NewSharedInformerFactoryWithOptions(kubeClient, resyncPeriod()(), informers.WithNamespace("openshift-machine-api")) - // Create scoped informer for MCO pods only (namespace + label filtered) - // This dramatically reduces memory overhead by only caching the machine-config-operator pod - // instead of all pods cluster-wide (potentially thousands of pods) - mcoPodInformer := informers.NewSharedInformerFactoryWithOptions( - kubeClient, - resyncPeriod()(), - informers.WithNamespace(MCONamespace), - informers.WithTweakListOptions(func(opt *metav1.ListOptions) { - opt.LabelSelector = labels.Set{"k8s-app": "machine-config-operator"}.String() - }), - ) imageSharedInformer := imageinformers.NewSharedInformerFactory(imageClient, resyncPeriod()()) routeSharedInformer := routeinformers.NewSharedInformerFactory(routeClient, resyncPeriod()()) @@ -167,7 +150,6 @@ func CreateControllerContext(ctx context.Context, cb *clients.Builder) *Controll OpenShiftConfigKubeNamespacedInformerFactory: openShiftConfigKubeNamespacedSharedInformer, OpenShiftKubeAPIServerKubeNamespacedInformerFactory: openShiftKubeAPIServerKubeNamespacedSharedInformer, OpenShiftConfigManagedKubeNamespacedInformerFactory: openShiftConfigManagedKubeNamespacedSharedInformer, - MCOPodInformerFactory: mcoPodInformer, APIExtInformerFactory: apiExtSharedInformer, ConfigInformerFactory: configSharedInformer, OperatorInformerFactory: operatorSharedInformer, @@ -211,25 +193,3 @@ func NewScopedNodeInformer(kubeclient kubernetes.Interface, nodeName string) (co func NewScopedNodeInformerFromClientBuilder(cb *clients.Builder, nodeName string) (corev1informers.NodeInformer, func(<-chan struct{})) { return NewScopedNodeInformer(cb.KubeClientOrDie("node-scoped-informer"), nodeName) } - -// Creates a scoped MachineConfigNode informer that only watches the MCN for -// a single node. Since the daemon only needs its own node's MCN, this avoids -// caching all MachineConfigNodes cluster-wide. Uses a metadata.name field -// selector to scope the list/watch to the given node name. Returns the -// instantiated MachineConfigNodeInformer and a start function. -func NewScopedMachineConfigNodeInformer(client mcfgclientset.Interface, nodeName string) (mcfginformersv1.MachineConfigNodeInformer, func(<-chan struct{})) { - sif := mcfginformers.NewSharedInformerFactoryWithOptions( - client, - resyncPeriod()(), - mcfginformers.WithTweakListOptions(func(opts *metav1.ListOptions) { - opts.FieldSelector = fields.OneTermEqualSelector("metadata.name", nodeName).String() - }), - ) - - return sif.Machineconfiguration().V1().MachineConfigNodes(), sif.Start -} - -// Creates a scoped MachineConfigNode informer from a clients.Builder instance. -func NewScopedMachineConfigNodeInformerFromClientBuilder(cb *clients.Builder, nodeName string) (mcfginformersv1.MachineConfigNodeInformer, func(<-chan struct{})) { - return NewScopedMachineConfigNodeInformer(cb.MachineConfigClientOrDie("mcn-scoped-informer"), nodeName) -} diff --git a/pkg/controller/common/layered_node_state.go b/pkg/controller/common/layered_node_state.go index 41b77fff7f..5b33c64b4b 100644 --- a/pkg/controller/common/layered_node_state.go +++ b/pkg/controller/common/layered_node_state.go @@ -19,19 +19,11 @@ import ( // status.go with this code, then repackage this so that it can be used by any // portion of the MCO which needs to interrogate or mutate node state. type LayeredNodeState struct { - node *corev1.Node - copied bool // true if we've already performed the safety DeepCopy + node *corev1.Node } -// NewLayeredNodeState creates a LayeredNodeState with copy-on-write semantics. -// The provided node may be from a lister cache; a DeepCopy is deferred until -// the first mutation via SetDesiredStateFromPool or -// SetDesiredStateFromMachineOSConfig. Node() returns the underlying pointer -// for read-only access; callers must not mutate it directly. -func NewLayeredNodeState(node *corev1.Node) *LayeredNodeState { - return &LayeredNodeState{ - node: node, - } +func NewLayeredNodeState(n *corev1.Node) *LayeredNodeState { + return &LayeredNodeState{node: n} } // Checks if the node is done "working." For a node in both layered and non-layered MCPs, the @@ -144,42 +136,42 @@ func (l *LayeredNodeState) isImageAnnotationEqualToMachineOSConfig(anno string, // Sets the desired MachineConfig annotations from the MachineConfigPool. // Deletes any desired image annotations if they exist. +// +// Note: This will create a deep copy of the node object first to avoid +// mutating any underlying caches. func (l *LayeredNodeState) SetDesiredStateFromPool(mcp *mcfgv1.MachineConfigPool) { - l.ensureSafeForMutation() + node := l.Node() + + metav1.SetMetaDataAnnotation(&node.ObjectMeta, daemonconsts.DesiredMachineConfigAnnotationKey, mcp.Spec.Configuration.Name) + delete(node.Annotations, daemonconsts.DesiredImageAnnotationKey) - metav1.SetMetaDataAnnotation(&l.node.ObjectMeta, daemonconsts.DesiredMachineConfigAnnotationKey, mcp.Spec.Configuration.Name) - delete(l.node.Annotations, daemonconsts.DesiredImageAnnotationKey) + l.node = node } // Sets the desired MachineConfig & image annotations from the MachineOSBuild and // MachineOSConfig objects. +// +// Note: This will create a deep copy of the node object first to avoid +// mutating any underlying caches. + func (l *LayeredNodeState) SetDesiredStateFromMachineOSConfig(mosc *mcfgv1.MachineOSConfig, mosb *mcfgv1.MachineOSBuild) { - l.ensureSafeForMutation() + node := l.Node() - metav1.SetMetaDataAnnotation(&l.node.ObjectMeta, daemonconsts.DesiredMachineConfigAnnotationKey, mosb.Spec.MachineConfig.Name) + metav1.SetMetaDataAnnotation(&node.ObjectMeta, daemonconsts.DesiredMachineConfigAnnotationKey, mosb.Spec.MachineConfig.Name) moscs := NewMachineOSConfigState(mosc) if moscs.HasOSImage() { - metav1.SetMetaDataAnnotation(&l.node.ObjectMeta, daemonconsts.DesiredImageAnnotationKey, moscs.GetOSImage()) + metav1.SetMetaDataAnnotation(&node.ObjectMeta, daemonconsts.DesiredImageAnnotationKey, moscs.GetOSImage()) } else { - delete(l.node.Annotations, daemonconsts.DesiredImageAnnotationKey) + delete(node.Annotations, daemonconsts.DesiredImageAnnotationKey) } -} -// Node returns the node object for read-only access. Callers must not mutate -// the returned pointer directly; use SetDesiredStateFromPool or -// SetDesiredStateFromMachineOSConfig instead. -func (l *LayeredNodeState) Node() *corev1.Node { - return l.node + l.node = node } -// ensureSafeForMutation performs a DeepCopy on the first mutation so that -// lister-cached nodes are never modified in place. -func (l *LayeredNodeState) ensureSafeForMutation() { - if !l.copied { - l.node = l.node.DeepCopy() - l.copied = true - } +// Returns a deep copy of the underlying node object. +func (l *LayeredNodeState) Node() *corev1.Node { + return l.node.DeepCopy() } // isNodeDone returns true if the current == desired and the MCD has marked done. diff --git a/pkg/controller/kubelet-config/helpers.go b/pkg/controller/kubelet-config/helpers.go index 837de0cb32..3faa986a70 100644 --- a/pkg/controller/kubelet-config/helpers.go +++ b/pkg/controller/kubelet-config/helpers.go @@ -440,20 +440,17 @@ func isControlPlanePool(pool *mcfgv1.MachineConfigPool) bool { return false } -// wrapErrorWithCondition converts an error into a KubeletConfigCondition with type KubeletConfigAccepted. -// If err is non-nil, the condition status is set to False; otherwise it is set to True. -// Optional args can provide a format string and values to override the default condition message. func wrapErrorWithCondition(err error, args ...interface{}) mcfgv1.KubeletConfigCondition { var condition *mcfgv1.KubeletConfigCondition if err != nil { condition = apihelpers.NewKubeletConfigCondition( - mcfgv1.KubeletConfigAccepted, - corev1.ConditionFalse, + mcfgv1.KubeletConfigFailure, + corev1.ConditionTrue, fmt.Sprintf("Error: %v", err), ) } else { condition = apihelpers.NewKubeletConfigCondition( - mcfgv1.KubeletConfigAccepted, + mcfgv1.KubeletConfigSuccess, corev1.ConditionTrue, "Success", ) @@ -467,17 +464,6 @@ func wrapErrorWithCondition(err error, args ...interface{}) mcfgv1.KubeletConfig return *condition } -// LEGACY: legacyConditionFromAccepted converts a KubeletConfigAccepted condition to the -// corresponding deprecated KubeletConfigSuccess or KubeletConfigFailure condition for backwards -// compatibility with downstream consumers (openshift/origin, openshift/oc). -// TODO(OCPNODE-4040): Remove once openshift/origin and openshift/oc use KubeletConfigAccepted. -func legacyConditionFromAccepted(cond mcfgv1.KubeletConfigCondition) mcfgv1.KubeletConfigCondition { - if cond.Status == corev1.ConditionTrue { - return *apihelpers.NewKubeletConfigCondition(mcfgv1.KubeletConfigSuccess, corev1.ConditionTrue, cond.Message) - } - return *apihelpers.NewKubeletConfigCondition(mcfgv1.KubeletConfigFailure, corev1.ConditionTrue, cond.Message) -} - func DecodeKubeletConfig(data []byte) (*kubeletconfigv1beta1.KubeletConfiguration, error) { config := &kubeletconfigv1beta1.KubeletConfiguration{} d := yaml.NewYAMLOrJSONDecoder(bytes.NewReader(data), len(data)) diff --git a/pkg/controller/kubelet-config/helpers_test.go b/pkg/controller/kubelet-config/helpers_test.go index e4360599af..c015655d7a 100644 --- a/pkg/controller/kubelet-config/helpers_test.go +++ b/pkg/controller/kubelet-config/helpers_test.go @@ -25,34 +25,34 @@ func TestWrapErrorWithCondition(t *testing.T) { expectedMessage string }{ { - name: "error without args produces KubeletConfigAccepted condition with status False", + name: "error without args produces Failure condition with status True", err: fmt.Errorf("KubeletConfiguration: swapBehavior is not allowed to be set, but contains: LimitedSwap"), args: nil, - expectedType: mcfgv1.KubeletConfigAccepted, - expectedStatus: corev1.ConditionFalse, + expectedType: mcfgv1.KubeletConfigFailure, + expectedStatus: corev1.ConditionTrue, expectedMessage: "Error: KubeletConfiguration: swapBehavior is not allowed to be set, but contains: LimitedSwap", }, { - name: "error with formatted args produces KubeletConfigAccepted condition with status False", + name: "error with formatted args produces Failure condition with status True", err: fmt.Errorf("validation failed"), args: []interface{}{"Failed to validate %s: %v", "kubelet config", "invalid field"}, - expectedType: mcfgv1.KubeletConfigAccepted, - expectedStatus: corev1.ConditionFalse, + expectedType: mcfgv1.KubeletConfigFailure, + expectedStatus: corev1.ConditionTrue, expectedMessage: "Failed to validate kubelet config: invalid field", }, { - name: "nil error produces KubeletConfigAccepted condition with status True", + name: "nil error produces Success condition with status True", err: nil, args: nil, - expectedType: mcfgv1.KubeletConfigAccepted, + expectedType: mcfgv1.KubeletConfigSuccess, expectedStatus: corev1.ConditionTrue, expectedMessage: "Success", }, { - name: "nil error with args still produces KubeletConfigAccepted condition with status True", + name: "nil error with args still produces Success condition", err: nil, args: []interface{}{"Custom success message"}, - expectedType: mcfgv1.KubeletConfigAccepted, + expectedType: mcfgv1.KubeletConfigSuccess, expectedStatus: corev1.ConditionTrue, expectedMessage: "Custom success message", }, @@ -77,37 +77,6 @@ func TestWrapErrorWithCondition(t *testing.T) { } } -func TestLegacyConditionFromAccepted(t *testing.T) { - tests := []struct { - name string - input mcfgv1.KubeletConfigCondition - expectedType mcfgv1.KubeletConfigStatusConditionType - expectedMsg string - }{ - { - name: "accepted True maps to Success True", - input: mcfgv1.KubeletConfigCondition{Type: mcfgv1.KubeletConfigAccepted, Status: corev1.ConditionTrue, Message: "Success"}, - expectedType: mcfgv1.KubeletConfigSuccess, - expectedMsg: "Success", - }, - { - name: "accepted False maps to Failure True", - input: mcfgv1.KubeletConfigCondition{Type: mcfgv1.KubeletConfigAccepted, Status: corev1.ConditionFalse, Message: "Error: validation failed"}, - expectedType: mcfgv1.KubeletConfigFailure, - expectedMsg: "Error: validation failed", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - legacy := legacyConditionFromAccepted(tt.input) - require.Equal(t, tt.expectedType, legacy.Type) - require.Equal(t, corev1.ConditionTrue, legacy.Status) - require.Equal(t, tt.expectedMsg, legacy.Message) - }) - } -} - // TestReserveSystemCPUs tests that when reservedSystemCPUs is set, // the systemReservedCgroup is cleared and system-reserved enforcement keys are removed // from enforceNodeAllocatable while preserving other user-provided values. diff --git a/pkg/controller/kubelet-config/kubelet_config_controller.go b/pkg/controller/kubelet-config/kubelet_config_controller.go index 26a856a240..0a6c19113c 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller.go @@ -467,7 +467,6 @@ func generateOriginalKubeletConfigIgn(cc *mcfgv1.ControllerConfig, templatesDir, return nil, fmt.Errorf("could not generate old kubelet config") } -// syncStatusOnly updates the status conditions of a KubeletConfig CR without modifying its spec. func (ctrl *Controller) syncStatusOnly(cfg *mcfgv1.KubeletConfig, err error, args ...interface{}) error { statusUpdateError := retry.RetryOnConflict(updateBackoff, func() error { newcfg, getErr := ctrl.mckLister.Get(cfg.Name) @@ -478,17 +477,13 @@ func (ctrl *Controller) syncStatusOnly(cfg *mcfgv1.KubeletConfig, err error, arg if newcfg.GetGeneration() != newcfg.Status.ObservedGeneration { newcfg.Status.ObservedGeneration = newcfg.GetGeneration() } + // Keeps a list of three status to avoid a long list of same statuses, + // only append a status if it is the first status + // or if the status message is different from the message of the last status recorded + // If the last status message is the same as the new one, then update the last status to + // reflect the latest time stamp from the new status message. newStatusCondition := wrapErrorWithCondition(err, args...) - - // LEGACY: Strip deprecated conditions before cleanup so the rolling window only tracks Accepted. - // TODO(OCPNODE-4040): Remove the next line once openshift/origin and openshift/oc use KubeletConfigAccepted. - removeLegacyConditions(&newcfg.Status.Conditions) - cleanUpStatusConditions(&newcfg.Status.Conditions, newStatusCondition) - - // LEGACY: Append deprecated KubeletConfigSuccess/KubeletConfigFailure for backwards compat. - // TODO(OCPNODE-4040): Remove the next line once openshift/origin and openshift/oc use KubeletConfigAccepted. - newcfg.Status.Conditions = append(newcfg.Status.Conditions, legacyConditionFromAccepted(newStatusCondition)) _, lerr := ctrl.client.MachineconfigurationV1().KubeletConfigs().UpdateStatus(context.TODO(), newcfg, metav1.UpdateOptions{}) return lerr }) @@ -512,19 +507,6 @@ func cleanUpStatusConditions(statusConditions *[]mcfgv1.KubeletConfigCondition, } } -// LEGACY: removeLegacyConditions strips deprecated KubeletConfigSuccess/KubeletConfigFailure conditions. -// Called before cleanUpStatusConditions so the rolling window only tracks KubeletConfigAccepted entries. -// TODO(OCPNODE-4040): Remove once openshift/origin and openshift/oc use KubeletConfigAccepted. -func removeLegacyConditions(conditions *[]mcfgv1.KubeletConfigCondition) { - filtered := (*conditions)[:0] - for _, c := range *conditions { - if c.Type != mcfgv1.KubeletConfigSuccess && c.Type != mcfgv1.KubeletConfigFailure { - filtered = append(filtered, c) - } - } - *conditions = filtered -} - // addAnnotation adds the annotions for a kubeletconfig object with the given annotationKey and annotationVal func (ctrl *Controller) addAnnotation(cfg *mcfgv1.KubeletConfig, annotationKey, annotationVal string) error { annotationUpdateErr := retry.RetryOnConflict(updateBackoff, func() error { @@ -656,9 +638,7 @@ func (ctrl *Controller) syncKubeletConfig(key string) error { } configSuccess := false if len(cfg.Status.Conditions) > 0 { - lastCondition := cfg.Status.Conditions[len(cfg.Status.Conditions)-1] - configSuccess = (lastCondition.Type == mcfgv1.KubeletConfigAccepted && lastCondition.Status == corev1.ConditionTrue) || - lastCondition.Type == mcfgv1.KubeletConfigSuccess // backwards compatibility + configSuccess = cfg.Status.Conditions[len(cfg.Status.Conditions)-1].Type == mcfgv1.KubeletConfigSuccess } if match && cfg.Status.ObservedGeneration >= cfg.Generation && configSuccess { // But we still need to compare the generated controller version because during an upgrade we need a new one diff --git a/pkg/controller/kubelet-config/kubelet_config_controller_test.go b/pkg/controller/kubelet-config/kubelet_config_controller_test.go index 8043498647..5fbbc85a4e 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller_test.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller_test.go @@ -1441,18 +1441,6 @@ func TestCleanUpStatusConditions(t *testing.T) { } } -func TestRemoveLegacyConditions(t *testing.T) { - conditions := []mcfgv1.KubeletConfigCondition{ - {Type: mcfgv1.KubeletConfigSuccess, Status: corev1.ConditionTrue, Message: "Success"}, - {Type: mcfgv1.KubeletConfigAccepted, Status: corev1.ConditionTrue, Message: "Success"}, - {Type: mcfgv1.KubeletConfigFailure, Status: corev1.ConditionTrue, Message: "Error"}, - } - - removeLegacyConditions(&conditions) - require.Equal(t, 1, len(conditions)) - assert.Equal(t, mcfgv1.KubeletConfigAccepted, conditions[0].Type) -} - func TestKubeletConfigResync(t *testing.T) { for _, platform := range []osev1.PlatformType{osev1.AWSPlatformType, osev1.NonePlatformType, "unrecognized"} { t.Run(string(platform), func(t *testing.T) { diff --git a/pkg/controller/node/node_controller.go b/pkg/controller/node/node_controller.go index cb4a2ed877..e45b5b9966 100644 --- a/pkg/controller/node/node_controller.go +++ b/pkg/controller/node/node_controller.go @@ -324,8 +324,8 @@ func (ctrl *Controller) Run(ctx context.Context, workers int) { syncers := []cache.InformerSynced{ ctrl.ccListerSynced, ctrl.mcListerSynced, ctrl.mcpListerSynced, ctrl.moscListerSynced, - ctrl.mosbListerSynced, ctrl.nodeListerSynced, ctrl.mcnListerSynced, ctrl.schedulerListerSynced, - ctrl.mcopListerSynced, ctrl.infraListerSynced, + ctrl.mosbListerSynced, ctrl.nodeListerSynced, ctrl.schedulerListerSynced, ctrl.mcopListerSynced, + ctrl.infraListerSynced, } // Only wait for the OSImageStream informer to sync if the feature is enabled if ctrl.osStreamsFgEnabled { diff --git a/pkg/controller/node/status.go b/pkg/controller/node/status.go index 530c7ecaba..9993a1f038 100644 --- a/pkg/controller/node/status.go +++ b/pkg/controller/node/status.go @@ -31,7 +31,7 @@ func (ctrl *Controller) syncStatusOnly(pool *mcfgv1.MachineConfigPool) error { machineConfigStates := []*mcfgv1.MachineConfigNode{} for _, node := range nodes { - ms, err := ctrl.mcnLister.Get(node.Name) + ms, err := ctrl.client.MachineconfigurationV1().MachineConfigNodes().Get(context.TODO(), node.Name, metav1.GetOptions{}) if err != nil { klog.Errorf("Could not find our MachineConfigNode for node. %s: %v", node.Name, err) continue diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index c26ccfbcca..758820c22d 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -96,8 +96,6 @@ const ( mccEventsRoleBindingTargetManifestPath = "manifests/machineconfigcontroller/events-rolebinding-target.yaml" mccConfigMapsRoleTargetManifestPath = "manifests/machineconfigcontroller/configmaps-role-target.yaml" mccConfigMapsRoleBindingTargetManifestPath = "manifests/machineconfigcontroller/configmaps-rolebinding-target.yaml" - mccInstallConfigRoleManifestPath = "manifests/machineconfigcontroller/install-config-role.yaml" - mccInstallConfigRoleBindingManifestPath = "manifests/machineconfigcontroller/install-config-rolebinding.yaml" mccClusterRoleBindingManifestPath = "manifests/machineconfigcontroller/clusterrolebinding.yaml" mccServiceAccountManifestPath = "manifests/machineconfigcontroller/sa.yaml" mccKubeRbacProxyConfigMapPath = "manifests/machineconfigcontroller/kube-rbac-proxy-config.yaml" @@ -345,34 +343,48 @@ func (optr *Operator) getImageRegistryBundles() ([]mcfgv1.ImageRegistryBundle, [ // Sync cloud config on supported platform from cloud.conf available in openshift-config-managed/kube-cloud-config ConfigMap. func (optr *Operator) syncCloudConfig(spec *mcfgv1.ControllerConfigSpec, infra *configv1.Infrastructure) error { - cm, err := optr.clusterCmLister.ConfigMaps("openshift-config-managed").Get("kube-cloud-config") - if err != nil { - if apierrors.IsNotFound(err) { - if isKubeCloudConfigCMRequired(infra) { - // Return error only if the kube-cloud-config ConfigMap is required, otherwise proceeds further. - return fmt.Errorf("%s/%s configmap is required on platform %s but not found: %w", - "openshift-config-managed", "kube-cloud-config", infra.Status.PlatformStatus.Type, err) + var lastErr error + if err := wait.PollUntilContextTimeout(context.TODO(), 1*time.Second, 3*time.Second, true, func(_ context.Context) (bool, error) { + cm, err := optr.clusterCmLister.ConfigMaps("openshift-config-managed").Get("kube-cloud-config") + if err != nil { + if apierrors.IsNotFound(err) { + if isKubeCloudConfigCMRequired(infra) { + // Return error only if the kube-cloud-config ConfigMap is required, otherwise proceeds further. + platformType := "Unknown" + if infra.Status.PlatformStatus != nil { + platformType = string(infra.Status.PlatformStatus.Type) + } + lastErr = fmt.Errorf("%s/%s configmap is required on platform %s but not found: %w", + "openshift-config-managed", "kube-cloud-config", platformType, err) + return false, nil + } + return true, nil } - return nil + lastErr = err + return false, nil } - return err - } - // Read cloud.conf from openshift-config-managed/kube-cloud-config ConfigMap. - cc, err := getCloudConfigFromConfigMap(cm, "cloud.conf") - if err != nil { - if isCloudConfRequired(infra) { - // Return error only if cloud.conf is required, otherwise proceeds further. - return fmt.Errorf("%s/%s configmap must have the %s key on platform %s but not found", - "openshift-config-managed", "kube-cloud-config", "cloud.conf", infra.Status.PlatformStatus.Type) + // Read cloud.conf from openshift-config-managed/kube-cloud-config ConfigMap. + cc, err := getCloudConfigFromConfigMap(cm, "cloud.conf") + if err != nil { + if isCloudConfRequired(infra) { + // Return error only if cloud.conf is required, otherwise proceeds further. + lastErr = fmt.Errorf("%s/%s configmap must have the %s key on platform %s but not found", + "openshift-config-managed", "kube-cloud-config", "cloud.conf", infra.Status.PlatformStatus.Type) + return false, nil + } + } else { + spec.CloudProviderConfig = cc } - } else { - spec.CloudProviderConfig = cc - } - caCert, err := ctrlcommon.GetCAsFromConfigMap(cm, "ca-bundle.pem") - if err == nil { - spec.CloudProviderCAData = caCert + caCert, err := ctrlcommon.GetCAsFromConfigMap(cm, "ca-bundle.pem") + if err == nil { + spec.CloudProviderCAData = caCert + } + return true, nil + }); err != nil { + return fmt.Errorf("during kube-cloud-config check: %w", kubeErrs.NewAggregate([]error{err, lastErr})) } + return nil } @@ -1195,14 +1207,12 @@ func (optr *Operator) syncMachineConfigController(config *renderConfig, _ *confi roles: []string{ mccKubeRbacProxyPrometheusRolePath, mccConfigMapsRoleTargetManifestPath, - mccInstallConfigRoleManifestPath, }, roleBindings: []string{ mccEventsRoleBindingDefaultManifestPath, mccEventsRoleBindingTargetManifestPath, mccKubeRbacProxyPrometheusRoleBindingPath, mccConfigMapsRoleBindingTargetManifestPath, - mccInstallConfigRoleBindingManifestPath, mopRoleBindingManifestPath, }, clusterRoleBindings: []string{ diff --git a/templates/common/_base/files/nmstate-configuration.yaml b/templates/common/_base/files/nmstate-configuration.yaml index 32dbbd592f..c1dcfc4e9b 100644 --- a/templates/common/_base/files/nmstate-configuration.yaml +++ b/templates/common/_base/files/nmstate-configuration.yaml @@ -29,7 +29,7 @@ contents: if [ -e "$src_path/applied" ]; then sha_applied=$(sha256sum "$src_path/applied" | awk '{print $1}') sha_config=$(sha256sum "$src_path/$config_file" | awk '{print $1}') - if [[ "$sha_applied" == "$sha_config" ]]; then + if ["$sha_applied"=="$sha_config"]; then echo "Configuration already applied, exiting" exit 0 fi diff --git a/templates/common/_base/units/machine-config-daemon-firstboot.service.yaml b/templates/common/_base/units/machine-config-daemon-firstboot.service.yaml index 7d67b39967..7ef8a4a81b 100644 --- a/templates/common/_base/units/machine-config-daemon-firstboot.service.yaml +++ b/templates/common/_base/units/machine-config-daemon-firstboot.service.yaml @@ -15,17 +15,9 @@ contents: | RemainAfterExit=yes # Disable existing repos (if any) so that OS extensions would use embedded RPMs only ExecStartPre=-/usr/bin/sh -c "sed -i 's/enabled=1/enabled=0/' /etc/yum.repos.d/*.repo" - # Bind mount a tmpfs-backed copy of the ostree repo config so that disabling fsync is ephemeral; - # if the host crashes or reboots the bind mount disappears and the on-disk config is unchanged. - # We append a second [core] section (GKeyFile merges duplicate groups) rather than using - # `ostree config set` because ostree uses an atomic rename which fails with EBUSY on a bind-mounted - # file; the bind mount must be established before `ostree config set` is called, and once it is, - # the rename target is a mount point and can't be replaced. - ExecStartPre=-/usr/bin/sh -c "cp /sysroot/ostree/repo/config /run/ostree-bootstrap-config && printf '\n[core]\nfsync = false\n' >> /run/ostree-bootstrap-config && mount --bind /run/ostree-bootstrap-config /sysroot/ostree/repo/config" # Run this via podman because we want to use the nmstatectl binary in our container ExecStart=/usr/bin/podman run --rm --privileged --net=host -v /:/rootfs --entrypoint machine-config-daemon '{{ .Images.machineConfigOperator }}' firstboot-complete-machineconfig --persist-nics ExecStart=/usr/bin/podman run --rm --privileged --pid=host --net=host -v /:/rootfs --entrypoint machine-config-daemon '{{ .Images.machineConfigOperator }}' firstboot-complete-machineconfig - ExecStopPost=-/usr/bin/umount /sysroot/ostree/repo/config {{if .Proxy -}} EnvironmentFile=/etc/mco/proxy.env {{end -}} diff --git a/test/extended-priv/kubeletconfig.go b/test/extended-priv/kubeletconfig.go index 5b919db65c..c28087a5bb 100644 --- a/test/extended-priv/kubeletconfig.go +++ b/test/extended-priv/kubeletconfig.go @@ -37,22 +37,20 @@ func (kc *KubeletConfig) create(parameters ...string) { exutil.CreateClusterResourceFromTemplate(kc.oc, allParams...) } -// waitUntilSuccess polls until the KubeletConfig reports a Accepted=True condition. func (kc KubeletConfig) waitUntilSuccess(timeout string) { logger.Infof("wait for %s to report success", kc.name) o.EventuallyWithOffset(1, &kc, timeout, "2s").Should(o.SatisfyAll( - HaveConditionField("Accepted", "status", "True"), - HaveConditionField("Accepted", "message", "Success"), - ), "KubeletConfig '%s' should report Accepted=True in status.conditions, but the current status is not success", kc.GetName()) + HaveConditionField("Success", "status", "True"), + HaveConditionField("Success", "message", "Success"), + ), "KubeletConfig '%s' should report Success in status.conditions, but the current status is not success", kc.GetName()) } -// waitUntilFailure polls until the KubeletConfig reports a Accepted=False condition with the expected message. func (kc KubeletConfig) waitUntilFailure(expectedMsg, timeout string) { logger.Infof("wait for %s to report failure", kc.name) o.EventuallyWithOffset(1, &kc, timeout, "2s").Should(o.SatisfyAll( - HaveConditionField("Accepted", "status", "False"), - HaveConditionField("Accepted", "message", o.ContainSubstring(expectedMsg)), - ), "KubeletConfig '%s' should report Accepted=False in status.conditions and report failure message %s. But it doesn't.", kc.GetName(), expectedMsg) + HaveConditionField("Failure", "status", "True"), + HaveConditionField("Failure", "message", o.ContainSubstring(expectedMsg)), + ), "KubeletConfig '%s' should report Failure in status.conditions and report failure message %s. But it doesn't.", kc.GetName(), expectedMsg) } // GetGeneratedMCName returns the name of the MC that was generated by this KubeletConfig resource diff --git a/test/extended-priv/machineconfigpool.go b/test/extended-priv/machineconfigpool.go index 77842b4a82..e4336ac3a8 100644 --- a/test/extended-priv/machineconfigpool.go +++ b/test/extended-priv/machineconfigpool.go @@ -135,6 +135,13 @@ func (mcp *MachineConfigPool) SetMaxUnavailable(maxUnavailable int) { o.Expect(err).NotTo(o.HaveOccurred()) } +// RemoveMaxUnavailable removes spec.maxUnavailable attribute from the pool config +func (mcp *MachineConfigPool) RemoveMaxUnavailable() { + logger.Infof("patch mcp %v, removing spec.maxUnavailable", mcp.name) + err := mcp.Patch("json", `[{ "op": "remove", "path": "/spec/maxUnavailable" }]`) + o.Expect(err).NotTo(o.HaveOccurred()) +} + // SetOsImageStream sets the osImageStream name for the MCP func (mcp *MachineConfigPool) SetOsImageStream(streamName string) error { logger.Infof("patch mcp %v, change spec.osImageStream.name to %s", mcp.name, streamName) @@ -151,13 +158,6 @@ func (mcp *MachineConfigPool) GetStatusOsImageStream() (string, error) { return mcp.Get(`{.status.osImageStream.name}`) } -// RemoveMaxUnavailable removes spec.maxUnavailable attribute from the pool config -func (mcp *MachineConfigPool) RemoveMaxUnavailable() { - logger.Infof("patch mcp %v, removing spec.maxUnavailable", mcp.name) - err := mcp.Patch("json", `[{ "op": "remove", "path": "/spec/maxUnavailable" }]`) - o.Expect(err).NotTo(o.HaveOccurred()) -} - func (mcp *MachineConfigPool) getConfigNameOfSpec() (string, error) { output, err := mcp.Get(`{.spec.configuration.name}`) logger.Infof("spec.configuration.name of mcp/%v is %v", mcp.name, output) @@ -537,80 +537,6 @@ func (mcp *MachineConfigPool) GetSortedNodesOrFail() []*Node { return nodes } -// GetSortedUpdatedNodes returns the list of the UpdatedNodes sorted by the time when they started to be updated. -// If maxUnavailable>0, then the function will fail if more that maxUpdatingNodes are being updated at the same time -func (mcp *MachineConfigPool) GetSortedUpdatedNodes(maxUnavailable int) []*Node { - timeToWait := mcp.estimateWaitDuration() - logger.Infof("Waiting %s in pool %s for all nodes to start updating.", timeToWait, mcp.name) - - poolNodes, errget := mcp.GetNodes() - o.Expect(errget).NotTo(o.HaveOccurred(), fmt.Sprintf("Cannot get nodes in pool %s", mcp.GetName())) - - pendingNodes := poolNodes - updatedNodes := []*Node{} - immediate := false - err := wait.PollUntilContextTimeout(context.TODO(), 20*time.Second, timeToWait, immediate, func(_ context.Context) (bool, error) { - // If there are degraded machines, stop polling, directly fail - degradedstdout, degradederr := mcp.getDegradedMachineCount() - if degradederr != nil { - logger.Errorf("the err:%v, and try next round", degradederr) - return false, nil - } - - if degradedstdout != 0 { - logger.Errorf("Degraded MC:\n%s", mcp.PrettyString()) - exutil.AssertWaitPollNoErr(fmt.Errorf("Degraded machines"), fmt.Sprintf("mcp %s has degraded %d machines", mcp.name, degradedstdout)) - } - - // Check that there aren't more thatn maxUpdatingNodes updating at the same time - if maxUnavailable > 0 { - totalUpdating := 0 - for _, node := range poolNodes { - isUpdating, err := node.IsUpdating() - if err != nil { - logger.Errorf("Error getting IsUpdating state for node %s: %v", node.GetName(), err) - return false, err - } - if isUpdating { - totalUpdating++ - } - } - if totalUpdating > maxUnavailable { - // print nodes for debug - mcp.oc.Run("get").Args("nodes").Execute() - exutil.AssertWaitPollNoErr(fmt.Errorf("maxUnavailable Not Honored. Pool %s, error: %d nodes were updating at the same time. Only %d nodes should be updating at the same time", mcp.GetName(), totalUpdating, maxUnavailable), "") - } - } - - remainingNodes := []*Node{} - for _, node := range pendingNodes { - isUpdating, err := node.IsUpdating() - if err != nil { - logger.Errorf("Error getting IsUpdating state for node %s: %v", node.GetName(), err) - return false, err - } - if isUpdating { - logger.Infof("Node %s is UPDATING", node.GetName()) - updatedNodes = append(updatedNodes, node) - } else { - remainingNodes = append(remainingNodes, node) - } - } - - if len(remainingNodes) == 0 { - logger.Infof("All nodes have started to be updated on mcp %s", mcp.name) - return true, nil - - } - logger.Infof(" %d remaining nodes", len(remainingNodes)) - pendingNodes = remainingNodes - return false, nil - }) - - exutil.AssertWaitPollNoErr(err, fmt.Sprintf("Could not get the list of updated nodes on mcp %s", mcp.name)) - return updatedNodes -} - // GetCordonedNodes get cordoned nodes (if maxUnavailable > 1 ) otherwise return the 1st cordoned node func (mcp *MachineConfigPool) GetCordonedNodes() []*Node { @@ -1362,38 +1288,6 @@ func CreateCustomMCPWithStreamByLabel(oc *exutil.CLI, name, label, osstream stri return CreateCustomMCPWithStreamByNodes(oc, name, osstream, customMcpNodes) } -// AddNodesToMachineConfigPool adds the given nodes to the specified MCP by labeling them -// and waits for the MCP to report the expected machine count and updated status -func AddNodesToMachineConfigPool(oc *exutil.CLI, mcpName string, nodes []*Node) error { - mcp := NewMachineConfigPool(oc, mcpName) - - currentNodes, err := mcp.GetNodes() - if err != nil { - return fmt.Errorf("error getting current nodes from %s: %w", mcpName, err) - } - expectedCount := len(currentNodes) + len(nodes) - - for _, n := range nodes { - err := n.AddLabel(fmt.Sprintf("node-role.kubernetes.io/%s", mcpName), "") - if err != nil { - return fmt.Errorf("error labeling node %s to add it to pool %s: %w", n.GetName(), mcpName, err) - } - logger.Infof("Node %s added to pool %s", n.GetName(), mcpName) - } - - err = mcp.WaitForMachineCount(expectedCount, 5*time.Minute) - if err != nil { - return fmt.Errorf("the %s MCP is not reporting the expected machine count: %w", mcpName, err) - } - - err = mcp.WaitImmediateForUpdatedStatus() - if err != nil { - return fmt.Errorf("the %s MCP is not updated: %w", mcpName, err) - } - - return nil -} - // CreateCustomMCP creates a new custom MCP with the given name and number of nodes // No osstream field is set - MCP will inherit from worker pool automatically // Nodes will be taken from the worker pool @@ -1750,6 +1644,112 @@ func GetPoolWithArchDifferentFromOrFail(oc *exutil.CLI, arch architecture.Archit return nil } +// AddNodesToMachineConfigPool adds nodes to an existing MCP by labeling them and waiting for the MCP to be updated +// This function is similar to CreateCustomMCPWithStreamByNodes but for adding nodes to existing pools +func AddNodesToMachineConfigPool(oc *exutil.CLI, mcpName string, nodes []*Node) error { + mcp := NewMachineConfigPool(oc, mcpName) + + currentNodes, err := mcp.GetNodes() + if err != nil { + return fmt.Errorf("error getting current nodes from %s: %w", mcpName, err) + } + expectedCount := len(currentNodes) + len(nodes) + + for _, n := range nodes { + err := n.AddLabel(fmt.Sprintf("node-role.kubernetes.io/%s", mcpName), "") + if err != nil { + return fmt.Errorf("error labeling node %s to add it to pool %s: %w", n.GetName(), mcpName, err) + } + logger.Infof("Node %s added to pool %s", n.GetName(), mcpName) + } + + err = mcp.WaitForMachineCount(expectedCount, 5*time.Minute) + if err != nil { + return fmt.Errorf("the %s MCP is not reporting the expected machine count: %w", mcpName, err) + } + + err = mcp.WaitImmediateForUpdatedStatus() + if err != nil { + return fmt.Errorf("the %s MCP is not updated: %w", mcpName, err) + } + + return nil +} + +// GetSortedUpdatedNodes returns a list of nodes in the order that they are being updated by the MCO +// If maxUnavailable>0, then the function will fail if more that maxUpdatingNodes are being updated at the same time +func (mcp *MachineConfigPool) GetSortedUpdatedNodes(maxUnavailable int) []*Node { + timeToWait := mcp.estimateWaitDuration() + logger.Infof("Waiting %s in pool %s for all nodes to start updating.", timeToWait, mcp.name) + + poolNodes, errget := mcp.GetNodes() + o.Expect(errget).NotTo(o.HaveOccurred(), fmt.Sprintf("Cannot get nodes in pool %s", mcp.GetName())) + + pendingNodes := poolNodes + updatedNodes := []*Node{} + immediate := false + err := wait.PollUntilContextTimeout(context.TODO(), 20*time.Second, timeToWait, immediate, func(_ context.Context) (bool, error) { + // If there are degraded machines, stop polling, directly fail + degradedstdout, degradederr := mcp.getDegradedMachineCount() + if degradederr != nil { + logger.Errorf("the err:%v, and try next round", degradederr) + return false, nil + } + + if degradedstdout != 0 { + logger.Errorf("Degraded MC:\n%s", mcp.PrettyString()) + exutil.AssertWaitPollNoErr(fmt.Errorf("degraded machines"), fmt.Sprintf("mcp %s has degraded %d machines", mcp.name, degradedstdout)) + } + + // Check that there aren't more thatn maxUpdatingNodes updating at the same time + if maxUnavailable > 0 { + totalUpdating := 0 + for _, node := range poolNodes { + isUpdating, err := node.IsUpdating() + if err != nil { + logger.Errorf("Error getting IsUpdating state for node %s: %v", node.GetName(), err) + return false, err + } + if isUpdating { + totalUpdating++ + } + } + if totalUpdating > maxUnavailable { + // print nodes for debug + mcp.oc.Run("get").Args("nodes").Execute() + exutil.AssertWaitPollNoErr(fmt.Errorf("maxUnavailable Not Honored. Pool %s, error: %d nodes were updating at the same time. Only %d nodes should be updating at the same time", mcp.GetName(), totalUpdating, maxUnavailable), "") + } + } + + remainingNodes := []*Node{} + for _, node := range pendingNodes { + isUpdating, err := node.IsUpdating() + if err != nil { + logger.Errorf("Error getting IsUpdating state for node %s: %v", node.GetName(), err) + return false, err + } + if isUpdating { + logger.Infof("Node %s is UPDATING", node.GetName()) + updatedNodes = append(updatedNodes, node) + } else { + remainingNodes = append(remainingNodes, node) + } + } + + if len(remainingNodes) == 0 { + logger.Infof("All nodes have started to be updated on mcp %s", mcp.name) + return true, nil + + } + logger.Infof(" %d remaining nodes", len(remainingNodes)) + pendingNodes = remainingNodes + return false, nil + }) + + exutil.AssertWaitPollNoErr(err, fmt.Sprintf("Could not get the list of updated nodes on mcp %s", mcp.name)) + return updatedNodes +} + // IsOCL returns true if the pool is using On Cluster Layering functionality func (mcp MachineConfigPool) IsOCL() (bool, error) { isOCLEnabled, err := IsFeaturegateEnabled(mcp.GetOC(), "OnClusterBuild") diff --git a/test/extended-priv/machineosconfig.go b/test/extended-priv/machineosconfig.go index 2c611f6ff6..6f35a00213 100644 --- a/test/extended-priv/machineosconfig.go +++ b/test/extended-priv/machineosconfig.go @@ -329,20 +329,6 @@ func (mosc MachineOSConfig) GetCurrentMachineOSBuild() (*MachineOSBuild, error) return NewMachineOSBuild(mosc.GetOC(), mosbName), nil } -// SetRenderedImagePushspec patches the MOSC resource in order to configure a new renderedImagePushspec -func (mosc MachineOSConfig) SetRenderedImagePushspec(rips string) error { - escaped, err := json.Marshal(rips) - if err != nil { - return err - } - return mosc.Patch("json", `[{"op":"replace","path":"/spec/renderedImagePushSpec","value":`+string(escaped)+`}]`) -} - -// GetRenderedImagePushspec returns the current valude of renderedImagePushspec -func (mosc MachineOSConfig) GetRenderedImagePushspec() (string, error) { - return mosc.Get(`{.spec.renderedImagePushSpec}`) -} - // SetContainerfiles sets the container files used by this MOSC func (mosc MachineOSConfig) SetContainerfiles(containerFiles []ContainerFile) error { containerFilesBytes, err := json.Marshal(containerFiles) @@ -486,12 +472,3 @@ func SkipTestIfCannotUseInternalRegistry(oc *exutil.CLI) { g.Skip("The internal registry cannot be used to store the osImage in this cluster. Skipping test case") } } - -// IsUsingInternalRegistry checks if the current image is from the internal registry -func (mosc MachineOSConfig) IsUsingInternalRegistry() (bool, error) { - currentImage, err := mosc.GetStatusCurrentImagePullSpec() - if err != nil { - return false, err - } - return strings.Contains(currentImage, InternalRegistrySvcURL), nil -} diff --git a/test/extended-priv/machineset.go b/test/extended-priv/machineset.go index 92a00bb795..c810ad724e 100644 --- a/test/extended-priv/machineset.go +++ b/test/extended-priv/machineset.go @@ -409,7 +409,6 @@ func (msl *MachineSetList) GetAll() ([]*MachineSet, error) { func (msl *MachineSetList) GetAllOrFail() []*MachineSet { allMs, err := msl.GetAll() o.ExpectWithOffset(1, err).NotTo(o.HaveOccurred(), "Error getting the list of existing MachineSets") - o.ExpectWithOffset(1, allMs).NotTo(o.BeEmpty(), "No MachineSets found in namespace %s", msl.GetNamespace()) return allMs } diff --git a/test/extended-priv/mco_bootimages_aws_marketplace.go b/test/extended-priv/mco_bootimages_aws_marketplace.go index e6823bb9f4..fd2deecbe3 100644 --- a/test/extended-priv/mco_bootimages_aws_marketplace.go +++ b/test/extended-priv/mco_bootimages_aws_marketplace.go @@ -45,8 +45,6 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/disruptive ) g.JustBeforeEach(func() { - // Skip if no MAPI MachineSets are available (e.g. compute machine management migrated to CAPI) - exutil.SkipIfNoMAPIMachineSets(oc.AsAdmin()) machineConfiguration = GetMachineConfiguration(oc.AsAdmin()) PreChecks(oc) diff --git a/test/extended-priv/mco_bootimages_skew.go b/test/extended-priv/mco_bootimages_skew.go index 44a12ba8a0..51f4a3935f 100644 --- a/test/extended-priv/mco_bootimages_skew.go +++ b/test/extended-priv/mco_bootimages_skew.go @@ -94,9 +94,6 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/disruptive exutil.SkipIfUnsupportedOSStreamLabel(oc.AsAdmin()) - // This test only supports clusters managing compute machines via legacy MAPI MachineSets - exutil.SkipIfNoMAPIMachineSets(oc.AsAdmin()) - // No opinion on skew enforcement for these platforms will result in Automatic mode o.Expect(machineConfiguration.RemoveSkew()).To(o.Succeed()) @@ -134,9 +131,6 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/disruptive exutil.SkipIfUnsupportedOSStreamLabel(oc.AsAdmin()) - // This test only supports clusters managing compute machines via legacy MAPI MachineSets - exutil.SkipIfNoMAPIMachineSets(oc.AsAdmin()) - // No opinion on skew enforcement for these platforms will result in Automatic mode o.Expect(machineConfiguration.RemoveSkew()).To(o.Succeed()) diff --git a/test/extended-priv/mco_extensions.go b/test/extended-priv/mco_extensions.go index 64c8933ef1..12aaaa4bb5 100644 --- a/test/extended-priv/mco_extensions.go +++ b/test/extended-priv/mco_extensions.go @@ -144,110 +144,4 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/longdurati validateMcpRenderDegraded(mc, mcp, expectedRDMessage, expectedRDReason) }) - - g.It("[PolarionID:89090][OTP] verifyExtensionsStaged detects missing staged deployment after applying extensions [Disruptive]", func() { - var ( - testID = GetCurrentTestPolarionIDNumber() - expectedNDMessage = "no staged deployment found after applying extensions" - ) - - exutil.By("Get a pool for testing") - mcp, cleanup, err := GetCompactCompatibleOrCustomPool(oc.AsAdmin(), 1) - defer cleanup() - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting pool for testing") - node := mcp.GetSortedNodesOrFail()[0] - logger.Infof("MCP: %s, node: %s", mcp.GetName(), node.GetName()) - logger.Infof("OK!\n") - - exutil.By("Replace rpm-ostree with fake script that silently succeeds on install/override") - o.Expect(ReplaceRpmOstree(node, generateTemplateAbsolutePath("rpm-ostree-fake-install-noop.sh"))).To(o.Succeed(), - "Failed to replace rpm-ostree on node %s", node.GetName()) - logger.Infof("OK!\n") - - exutil.By("Apply MachineConfig with usbguard extension") - mc := NewMachineConfig(oc.AsAdmin(), fmt.Sprintf("test-%s-ext", testID), mcp.GetName()). - SetMCOTemplate("change-worker-extension-usbguard.yaml") - mc.skipWaitForMcp = true - defer func() { - exutil.By("Restore rpm-ostree, delete MC, and recover MCP") - o.Expect(RestoreRpmOstree(node)).To(o.Succeed(), "Failed to restore rpm-ostree on node %s", node.GetName()) - o.Eventually(mc.Delete).Should(o.Succeed(), "Could not delete the extension MC") - o.Expect(mcp.RecoverFromDegraded()).To(o.Succeed(), "The MCP could not be recovered from Degraded status") - }() - mc.create() - logger.Infof("OK!\n") - - exutil.By("Restart MCD pod on the node to pick up fake rpm-ostree") - mcdPod := node.GetMachineConfigDaemon() - err = NewNamespacedResource(oc.AsAdmin(), "pod", MachineConfigNamespace, mcdPod).Delete() - o.Expect(err).NotTo(o.HaveOccurred(), "Failed to delete MCD pod %s", mcdPod) - logger.Infof("Deleted MCD pod %s to trigger re-sync with fake rpm-ostree", mcdPod) - logger.Infof("OK!\n") - - exutil.By("Wait for MCP to degrade with missing staged deployment error") - o.Eventually(mcp, mcp.estimateWaitDuration().String(), "30s").Should(BeDegraded(), - "The '%s' MCP should become degraded when no staged deployment is found after applying extensions", mcp.GetName()) - o.Expect(mcp).To(HaveNodeDegradedMessage(o.ContainSubstring(expectedNDMessage)), - "The '%s' MCP should report the staged deployment error in the NodeDegraded condition", mcp.GetName()) - logger.Infof("OK!\n") - }) - - g.It("[PolarionID:89095][OTP] verifyExtensionPackages detects extension missing from RPM database after reboot [Disruptive]", func() { - var ( - testID = GetCurrentTestPolarionIDNumber() - expectedNDMessage = "extension package verification failed" - fakeRpmLocalPath = generateTemplateAbsolutePath("rpm-fake-usbguard-missing.sh") - ) - - exutil.By("Get a pool for testing") - mcp, cleanup, err := GetCompactCompatibleOrCustomPool(oc.AsAdmin(), 1) - defer cleanup() - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting pool for testing") - node := mcp.GetSortedNodesOrFail()[0] - logger.Infof("MCP: %s, node: %s", mcp.GetName(), node.GetName()) - logger.Infof("OK!\n") - - exutil.By("Apply MachineConfig with usbguard extension") - mc := NewMachineConfig(oc.AsAdmin(), fmt.Sprintf("test-%s-ext", testID), mcp.GetName()). - SetMCOTemplate("change-worker-extension-usbguard.yaml") - mc.skipWaitForMcp = true - defer func() { - exutil.By("Restore rpm, delete MC, and recover MCP") - o.Expect(RestoreRpm(node)).To(o.Succeed(), "Failed to restore rpm on node %s", node.GetName()) - o.Eventually(mc.Delete).Should(o.Succeed(), "Could not delete the extension MC") - o.Expect(mcp.RecoverFromDegraded()).To(o.Succeed(), "The MCP could not be recovered from Degraded status") - }() - mc.create() - logger.Infof("OK!\n") - - exutil.By("Wait for node to reboot and MCP to finish updating") - o.Expect(mcp.WaitForUpdatedStatus()).To(o.Succeed(), - "The MCP should complete the update after installing usbguard extension") - logger.Infof("OK!\n") - - exutil.By("Re-apply fake rpm after reboot (bind mount is lost on reboot)") - o.Expect(ReplaceRpm(node, fakeRpmLocalPath)).To(o.Succeed(), - "Failed to re-apply fake rpm on node %s", node.GetName()) - logger.Infof("OK!\n") - - exutil.By("Restart MCD pod on the node to pick up fake rpm") - mcdPod := node.GetMachineConfigDaemon() - err = NewNamespacedResource(oc.AsAdmin(), "pod", MachineConfigNamespace, mcdPod).Delete() - o.Expect(err).NotTo(o.HaveOccurred(), "Failed to delete MCD pod %s", mcdPod) - logger.Infof("Deleted MCD pod %s to trigger re-sync", mcdPod) - logger.Infof("OK!\n") - - exutil.By("Wait for MCP to degrade with extension verification error") - o.Eventually(mcp, mcp.estimateWaitDuration().String(), "30s").Should(BeDegraded(), - "The '%s' MCP should become degraded when extension packages are missing from the RPM database", mcp.GetName()) - o.Expect(mcp).To(HaveNodeDegradedMessage(o.ContainSubstring(expectedNDMessage)), - "The '%s' MCP should report the extension verification error in the NodeDegraded condition", mcp.GetName()) - logger.Infof("OK!\n") - - exutil.By("Check MCD logs for extension verification error") - o.Eventually(node.GetMCDaemonLogs, "2m", "10s").WithArguments("").Should( - o.ContainSubstring(expectedNDMessage), - "MCD logs should contain the extension verification error") - logger.Infof("OK!\n") - }) }) diff --git a/test/extended-priv/mco_irreconcilablechanges.go b/test/extended-priv/mco_irreconcilablechanges.go index 148335e843..0ee64cc628 100644 --- a/test/extended-priv/mco_irreconcilablechanges.go +++ b/test/extended-priv/mco_irreconcilablechanges.go @@ -2,7 +2,6 @@ package extended import ( "fmt" - "strings" g "github.com/onsi/ginkgo/v2" o "github.com/onsi/gomega" @@ -77,25 +76,6 @@ func platformBasedDisksNames(platform string) []string { return []string{} } -// discoverNVMeByPathDisks discovers the /dev/disk/by-path/ entries for non-boot NVMe -// devices on an AWS node. These PCI-based paths are stable per instance type, unlike -// /dev/nvmeXn1 which depends on device enumeration order. -func discoverNVMeByPathDisks(node *Node) []string { - script := `for p in /dev/disk/by-path/*nvme*; do [[ $p == *part* ]] && continue; ls ${p}-part* &>/dev/null && continue; echo "$p"; done | sort` - stdout, _, err := node.DebugNodeWithChrootStd("bash", "-c", script) - o.ExpectWithOffset(1, err).NotTo(o.HaveOccurred(), "Failed to discover NVMe by-path disks on node %s", node.GetName()) - - var paths []string - for _, line := range strings.Split(strings.TrimSpace(stdout), "\n") { - line = strings.TrimSpace(line) - if line != "" { - paths = append(paths, line) - } - } - logger.Infof("Discovered %d non-boot NVMe by-path disks on node %s: %v", len(paths), node.GetName(), paths) - return paths -} - var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/disruptive][Disruptive][OCPFeatureGate:IrreconcilableMachineConfig][Serial]", g.Ordered, func() { defer g.GinkgoRecover() @@ -163,14 +143,17 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/disruptive logger.Infof("All worker nodes have irreconcilable changes as expected!\n") }) - g.It("[PolarionID:84219] Verify irreconcilable changes on new and existing nodes after scale up [Disruptive]", g.Label("Lifecycle:informing", "Platform:aws", "Platform:gce", "Platform:azure", "Platform:vsphere"), func() { + g.It("[PolarionID:84219] Verify irreconcilable changes on new and existing nodes after scale up [Disruptive]", g.Label("Platform:gce", "Platform:azure"), func() { var ( machineconfiguration = GetMachineConfiguration(oc) mcName = "irreconcilable-scaleup-test" initialMcSpecs = machineconfiguration.GetSpecOrFail() ) - SkipTestIfWorkersCannotBeScaled(oc) + // Only GCPPlatform and AzurePlatform are able to enumerate disks in a reliable manner right now. + // See https://redhat.atlassian.net/browse/MCO-2470 + skipTestIfSupportedPlatformNotMatched(oc, GCPPlatform, AzurePlatform) + mcp := NewMachineConfigPool(oc, MachineConfigPoolWorker) defer func() { @@ -204,25 +187,7 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/disruptive err = platformBasedDisksPatch(platform, newMS) o.Expect(err).NotTo(o.HaveOccurred()) - // On AWS, NVMe device enumeration is non-deterministic. Scale up a probe - // node first to discover the stable /dev/disk/by-path/ entries, then use - // those paths in the MachineConfig so Ignition can reliably find the disks. disks := platformBasedDisksNames(platform) - var probeNode *Node - if platform == AWSPlatform { - exutil.By("Step 2.5: Scale up probe node to discover disk paths") - o.Expect(newMS.ScaleTo(1)).To(o.Succeed()) - o.Expect(newMS.WaitUntilReady("10m")).To(o.Succeed()) - - probeNodes := newMS.GetNodesOrFail() - o.Expect(probeNodes).To(o.HaveLen(1)) - probeNode = probeNodes[0] - logger.Infof("Probe node is: %s", probeNode.GetName()) - - discoveredDisks := discoverNVMeByPathDisks(probeNode) - o.Expect(discoveredDisks).To(o.HaveLen(2), "Expected exactly 2 non-boot NVMe disks on probe node %s", probeNode.GetName()) - disks = discoveredDisks - } mc := NewMachineConfig(oc, mcName, MachineConfigPoolWorker).SetMCOTemplate("extra-disks-with-files.yaml") @@ -254,27 +219,12 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/disruptive logger.Infof("All worker nodes report irreconcilable storage changes!\n") exutil.By("Step 5: Scale up test node with MC applied via Ignition") - var testNode *Node - if platform == AWSPlatform { - o.Expect(newMS.ScaleTo(2)).To(o.Succeed()) - o.Expect(newMS.WaitUntilReady("10m")).To(o.Succeed()) - - allNodes := newMS.GetNodesOrFail() - o.Expect(allNodes).To(o.HaveLen(2)) - for _, n := range allNodes { - if n.GetName() != probeNode.GetName() { - testNode = n - } - } - o.Expect(testNode).NotTo(o.BeNil(), "Could not identify test node among scaled-up nodes") - } else { - o.Expect(newMS.ScaleTo(1)).To(o.Succeed()) - o.Expect(newMS.WaitUntilReady("10m")).To(o.Succeed()) - - testNodes := newMS.GetNodesOrFail() - o.Expect(testNodes).To(o.HaveLen(1)) - testNode = testNodes[0] - } + o.Expect(newMS.ScaleTo(1)).To(o.Succeed()) + o.Expect(newMS.WaitUntilReady("10m")).To(o.Succeed()) + + testNodes := newMS.GetNodesOrFail() + o.Expect(testNodes).To(o.HaveLen(1)) + testNode := testNodes[0] logger.Infof("Test node is: %s", testNode.GetName()) exutil.By("Step 6: Verify test node has no irreconcilable changes") diff --git a/test/extended-priv/mco_machineconfigpool.go b/test/extended-priv/mco_machineconfigpool.go index 8bed4c0f12..85dd389e6d 100644 --- a/test/extended-priv/mco_machineconfigpool.go +++ b/test/extended-priv/mco_machineconfigpool.go @@ -107,25 +107,15 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/longdurati g.It("[PolarionID:52373][OTP] Modify proxy configuration in paused pools", func() { - var ( - proxyValue = "http://user:pass@proxy-fake:1111" - noProxyValue = "test.52373.no-proxy.com" - expectedDegradedMessage = `required MachineConfigPool master is paused and cannot sync until it is unpaused` - wmcp = NewMachineConfigPool(oc.AsAdmin(), MachineConfigPoolWorker) - mmcp = NewMachineConfigPool(oc.AsAdmin(), MachineConfigPoolMaster) - machineConfiguration = GetMachineConfiguration(oc.AsAdmin()) - proxy = NewResource(oc.AsAdmin(), "proxy", "cluster") - mcoMCD = NewNamespacedResource(oc.AsAdmin(), "DaemonSet", MachineConfigNamespace, "machine-config-daemon") - mcoMCC = NewNamespacedResource(oc.AsAdmin(), "Deployment", MachineConfigNamespace, "machine-config-controller") - mco = NewResource(oc.AsAdmin(), "co", "machine-config") - kubeAPIServer = NewResource(oc.AsAdmin(), "co", "kube-apiserver") - ) + proxyValue := "http://user:pass@proxy-fake:1111" + noProxyValue := "test.52373.no-proxy.com" // Disable boot images update and skew enforcement so that the boot image controller // does not try to reconcile machinesets while the proxy is set to a fake value, which // would cause the controller to fail and degrade the cluster operator exutil.By("Disable boot images update and skew enforcement") if IsBootImageUpdateSupported(oc.AsAdmin()) { + machineConfiguration := GetMachineConfiguration(oc.AsAdmin()) defer machineConfiguration.SetSpec(machineConfiguration.GetSpecOrFail()) DisableSkew(machineConfiguration) o.Expect( @@ -137,31 +127,25 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/longdurati logger.Infof("OK!\n") exutil.By("Get current proxy configuration") - proxyInitialConfig := proxy.GetSpecOrFail() + proxy := NewResource(oc.AsAdmin(), "proxy", "cluster") + proxyInitialConfig := proxy.GetOrFail(`{.spec}`) logger.Infof("Initial proxy configuration: %s", proxyInitialConfig) - logger.Infof("OK!\n") - defer SafeCleanup(func() { + wmcp := NewMachineConfigPool(oc.AsAdmin(), MachineConfigPoolWorker) + mmcp := NewMachineConfigPool(oc.AsAdmin(), MachineConfigPoolMaster) + + defer func() { logger.Infof("Start TC defer block") logger.Infof("Restore original proxy config %s", proxyInitialConfig) - o.Expect(proxy.SetSpec(proxyInitialConfig)).To(o.Succeed()) + _ = proxy.Patch("json", `[{ "op": "add", "path": "/spec", "value": `+proxyInitialConfig+`}]`) logger.Infof("Wait for new machine configs to be rendered and paused pools to report updated status") // We need to make sure that the config will NOT be applied, since the proxy is a fake one and if // we dont make sure that the config proxy is reverted, the nodes will be broken and go into // NotReady status - o.Expect(wmcp.WaitForUpdatedStatus()).To(o.Succeed()) - o.Expect(mmcp.WaitForUpdatedStatus()).To(o.Succeed()) - - // Proxy changes trigger a kube-apiserver rollout. Wait for it to complete so that - // ValidatingAdmissionPolicy paramRef lookups don't fail while apiserver pods are replaced. - // Before modifying the MCP again, we need to make sure that the api server can handle the ValidatingAdmissionPolicy - exutil.By("Wait for kube-apiserver rollout to complete after proxy restore") - o.Eventually(kubeAPIServer, "20m", "30s").Should( - HaveConditionField("Progressing", "status", "False"), - "kube-apiserver is still progressing after proxy change") - logger.Infof("OK!\n") + _ = wmcp.WaitForUpdatedStatus() + _ = mmcp.WaitForUpdatedStatus() logger.Infof("Unpause worker pool") wmcp.pause(false) @@ -170,64 +154,60 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/longdurati mmcp.pause(false) logger.Infof("End TC defer block") - }) + }() exutil.By("Pause MCPs") wmcp.pause(true) mmcp.pause(true) - logger.Infof("OK!\n") exutil.By("Configure new proxy") err := proxy.Patch("json", - `[{ "op": "add", "path": "/spec/httpProxy", "value": "`+proxyValue+`" },`+ - `{ "op": "add", "path": "/spec/httpsProxy", "value": "`+proxyValue+`" },`+ - `{ "op": "add", "path": "/spec/noProxy", "value": "`+noProxyValue+`" }]`) - o.Expect(err).ShouldNot(o.HaveOccurred(), "Error patching proxy configuration") - logger.Infof("OK!\n") + `[{ "op": "add", "path": "/spec/httpProxy", "value": "`+proxyValue+`" }]`) + o.Expect(err).ShouldNot(o.HaveOccurred(), "Error patching http proxy") + + err = proxy.Patch("json", + `[{ "op": "add", "path": "/spec/httpsProxy", "value": "`+proxyValue+`" }]`) + o.Expect(err).ShouldNot(o.HaveOccurred(), "Error patching https proxy") - exutil.By("Verify that the proxy configuration was applied to the MCD daemonset and the MCC deployment") + err = proxy.Patch("json", + `[{ "op": "add", "path": "/spec/noProxy", "value": "`+noProxyValue+`" }]`) + o.Expect(err).ShouldNot(o.HaveOccurred(), "Error patching noproxy") + + exutil.By("Verify that the proxy configuration was applied to daemonsets") + mcoDs := NewNamespacedResource(oc.AsAdmin(), "DaemonSet", MachineConfigNamespace, "machine-config-daemon") // it should never take longer than 5 minutes to apply the proxy config under any circumstance, // it should be considered a bug. - o.Eventually(mcoMCD.GetSpec, "5m", "30s").Should(o.ContainSubstring(proxyValue), + o.Eventually(mcoDs.Poll(`{.spec}`), "5m", "30s").Should(o.ContainSubstring(proxyValue), "machine-config-daemon is not using the new proxy configuration: %s", proxyValue) - o.Eventually(mcoMCD.GetSpec, "5m", "30s").Should(o.ContainSubstring(noProxyValue), + o.Eventually(mcoDs.Poll(`{.spec}`), "5m", "30s").Should(o.ContainSubstring(noProxyValue), "machine-config-daemon is not using the new no-proxy value: %s", noProxyValue) - o.Eventually(mcoMCC.GetSpec, "5m", "30s").Should(o.ContainSubstring(proxyValue), - "machine-config-controller is not using the new proxy configuration: %s", proxyValue) - o.Eventually(mcoMCC.GetSpec, "5m", "30s").Should(o.ContainSubstring(noProxyValue), - "machine-config-controller is not using the new no-proxy value: %s", noProxyValue) - logger.Infof("OK!\n") exutil.By("Check that the operator has been marked as degraded") - o.Eventually(mco, "5m", "30s").Should(BeDegraded(), + mco := NewResource(oc.AsAdmin(), "co", "machine-config") + o.Eventually(mco.Poll(`{.status.conditions[?(@.type=="Degraded")].status}`), + "5m", "30s").Should(o.Equal("True"), "machine-config Operator should report degraded status") - o.Eventually(mco, "5m", "30s").Should(HaveDegradedMessage(o.ContainSubstring(expectedDegradedMessage)), + o.Eventually(mco.Poll(`{.status.conditions[?(@.type=="Degraded")].message}`), + "5m", "30s").Should(o.ContainSubstring(`required MachineConfigPool master is paused and cannot sync until it is unpaused`), "machine-config Operator is not reporting the right reason for degraded status") - logger.Infof("OK!\n") exutil.By("Restore original proxy configuration") - err = proxy.SetSpec(proxyInitialConfig) + err = proxy.Patch("json", `[{ "op": "add", "path": "/spec", "value": `+proxyInitialConfig+`}]`) o.Expect(err).ShouldNot(o.HaveOccurred(), "Error patching and restoring original proxy config") - logger.Infof("OK!\n") - exutil.By("Verify that the original configuration is restored in the MCD daemonset and the MCC deployment") + exutil.By("Verify that the new configuration is applied to the daemonset") // it should never take longer than 5 minutes to apply the proxy config under any circumstance, // it should be considered a bug. - o.Eventually(mcoMCD.GetSpec, "5m", "30s").ShouldNot(o.ContainSubstring(proxyValue), + o.Eventually(mcoDs.Poll(`{.spec}`), "5m", "30s").ShouldNot(o.ContainSubstring(proxyValue), "machine-config-daemon has not restored the original proxy configuration") - o.Eventually(mcoMCD.GetSpec, "5m", "30s").ShouldNot(o.ContainSubstring(noProxyValue), + o.Eventually(mcoDs.Poll(`{.spec}`), "5m", "30s").ShouldNot(o.ContainSubstring(noProxyValue), "machine-config-daemon has not restored the original proxy configuration for 'no-proxy'") - o.Eventually(mcoMCC.GetSpec, "5m", "30s").ShouldNot(o.ContainSubstring(proxyValue), - "machine-config-controller has not restored the original proxy configuration") - o.Eventually(mcoMCC.GetSpec, "5m", "30s").ShouldNot(o.ContainSubstring(noProxyValue), - "machine-config-controller has not restored the original proxy configuration for 'no-proxy'") - logger.Infof("OK!\n") exutil.By("Check that the operator is not marked as degraded anymore") - o.Eventually(mco, "5m", "30s").ShouldNot(BeDegraded(), + o.Eventually(mco.Poll(`{.status.conditions[?(@.type=="Degraded")].status}`), + "5m", "30s").Should(o.Equal("False"), "machine-config Operator should not report degraded status anymore") - logger.Infof("OK!\n") }) diff --git a/test/extended-priv/mco_ocb.go b/test/extended-priv/mco_ocb.go index 79e3c09ef0..3c5fb25924 100644 --- a/test/extended-priv/mco_ocb.go +++ b/test/extended-priv/mco_ocb.go @@ -426,8 +426,7 @@ func ValidateSuccessfulMOSC(mosc *MachineOSConfig, checkers []Checker) { logger.Infof("OK!\n") exutil.By("Check that a new build is successfully executed") - // When building several builds at the same time the pods can take more than 20 minutes to build a simple image - o.Eventually(mosb, "35m", "20s").Should(HaveConditionField("Building", "status", FalseString), "Build was not finished") + o.Eventually(mosb, "20m", "20s").Should(HaveConditionField("Building", "status", FalseString), "Build was not finished") o.Eventually(mosb, "10m", "20s").Should(HaveConditionField("Succeeded", "status", TrueString), "Build didn't succeed") o.Eventually(mosb, "2m", "20s").Should(HaveConditionField("Interrupted", "status", FalseString), "Build was interrupted") o.Eventually(mosb, "2m", "20s").Should(HaveConditionField("Failed", "status", FalseString), "Build was failed") @@ -700,9 +699,8 @@ func checkUsbguradExtension(node *Node) { rpmName = AllExtenstions[extName] activeString = "Active: active (running)" inactiveString = "Active: inactive (dead)" - expectedError = "missing /usr/lib/tmpfiles.d/usbguard.conf\nerror: non-zero exit code from debug container" - expectedErrorOCL = ".M....... /var/log/usbguard\nerror: non-zero exit code from debug container" - expectedErrorCombined = "missing /usr/lib/tmpfiles.d/usbguard.conf\n.M....... /var/log/usbguard\nerror: non-zero exit code from debug container" + expectedError = "missing /usr/lib/tmpfiles.d/usbguard.conf\nerror: non-zero exit code from debug container" + expectedErrorOCL = ".M....... /var/log/usbguard\nerror: non-zero exit code from debug container" ) exutil.By("Verify node includes Usbguard extension") o.Expect( @@ -731,7 +729,7 @@ func checkUsbguradExtension(node *Node) { exutil.By("Check that all the files in the rpm were correctly deployed ") rpmOut, _ := node.checkRpmFiles(rpmName...) - o.Expect(strings.TrimSpace(rpmOut)).Should(o.Or(o.BeEmpty(), o.Equal(expectedErrorOCL), o.Equal(expectedError), o.Equal(expectedErrorCombined)), "Error in permissions of rpm files") + o.Expect(strings.TrimSpace(rpmOut)).Should(o.Or(o.BeEmpty(), o.Equal(expectedErrorOCL), o.Equal(expectedError)), "Error in permissions of rpm files") logger.Infof("OK!\n") } diff --git a/test/extended-priv/mco_ocb_longduration.go b/test/extended-priv/mco_ocb_longduration.go deleted file mode 100644 index 8b3085598d..0000000000 --- a/test/extended-priv/mco_ocb_longduration.go +++ /dev/null @@ -1,1143 +0,0 @@ -package extended - -import ( - "fmt" - "os" - "path/filepath" - "strings" - "sync" - "time" - - g "github.com/onsi/ginkgo/v2" - o "github.com/onsi/gomega" - exutil "github.com/openshift/machine-config-operator/test/extended-priv/util" - logger "github.com/openshift/machine-config-operator/test/extended-priv/util/logext" -) - -var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/longduration][Serial][Disruptive] MCO ocb longduration", func() { - defer g.GinkgoRecover() - - var ( - oc = exutil.NewCLI("mco-ocb-longduration", exutil.KubeConfigPath()) - ) - - g.JustBeforeEach(func() { - PreChecks(oc) - SkipTestIfOCBIsEnabled(oc) - }) - - g.It("[PolarionID:83137][OTP][Skipped:Disconnected] OCB use OutputImage CurrentImagePullSecret [Disruptive]", func() { - var ( - mcp = GetCompactCompatiblePool(oc.AsAdmin()) - tmpNamespaceName = fmt.Sprintf("tc-%s-mco-ocl-images", GetCurrentTestPolarionIDNumber()) - checkers = []Checker{ - CommandOutputChecker{ - Command: []string{"rpm-ostree", "status"}, - Matcher: o.ContainSubstring(fmt.Sprintf("%s/%s/ocb-%s-image", InternalRegistrySvcURL, tmpNamespaceName, mcp.GetName())), - ErrorMsg: fmt.Sprintf("The nodes are not using the expected OCL image stored in the internal registry"), - Desc: fmt.Sprintf("Check that the nodes are using the right OS image"), - }, - } - ) - - testContainerFile([]ContainerFile{}, tmpNamespaceName, mcp, checkers, false) - }) - - g.It("[PolarionID:79137][OTP][Skipped:Disconnected] OCB Opting into on-cluster builds must respect maxUnavailable setting. Workers.[Disruptive]", func() { - SkipIfCompactOrSNO(oc.AsAdmin()) // This test makes no sense in SNO or Compact - - var ( - wMcp = NewMachineConfigPool(oc.AsAdmin(), MachineConfigPoolWorker) - // MOSC has to use the same name as the mcp - moscName = wMcp.GetName() - workerNodes = wMcp.GetSortedNodesOrFail() - ) - - exutil.By("Configure maxUnavailable if worker pool has more than 2 nodes") - if len(workerNodes) > 2 { - defer wMcp.SetSpec(wMcp.GetSpecOrFail()) - wMcp.SetMaxUnavailable(2) - } - - maxUnavailable := OrFail[int](wMcp.GetMaxUnavailableInt()) - logger.Infof("Current maxUnavailable value %d", maxUnavailable) - logger.Infof("OK!\n") - - exutil.By("Configure OCB functionality for the new worker MCP") - mosc, err := CreateMachineOSConfigUsingExternalOrInternalRegistry(oc.AsAdmin(), MachineConfigNamespace, moscName, wMcp.GetName(), nil) - defer DisableOCL(mosc) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating the MachineOSConfig resource") - logger.Infof("OK!\n") - - exutil.By("Wait for the worker MCP to start updating") - o.Eventually(wMcp.GetUpdatingStatus, "15m", "15s").Should(o.Equal("True"), - "The worker MCP did not start updating") - logger.Infof("OK!\n") - - exutil.By("Poll the nodes sorted by the order they are updated") - updatedNodes := wMcp.GetSortedUpdatedNodes(maxUnavailable) - for _, n := range updatedNodes { - logger.Infof("updated node: %s created: %s zone: %s", n.GetName(), n.GetOrFail(`{.metadata.creationTimestamp}`), n.GetOrFail(`{.metadata.labels.topology\.kubernetes\.io/zone}`)) - } - logger.Infof("OK!\n") - - exutil.By("Wait for the configuration to be applied in all nodes") - wMcp.waitForComplete() - logger.Infof("OK!\n") - - exutil.By("Check that nodes were updated in the right order") - rightOrder := checkUpdatedLists(workerNodes, updatedNodes, maxUnavailable) - o.Expect(rightOrder).To(o.BeTrue(), "Expected update order %s, but found order %s", workerNodes, updatedNodes) - logger.Infof("OK!\n") - - exutil.By("Remove the MachineOSConfig resource") - o.Expect(DisableOCL(mosc)).To(o.Succeed(), "Error cleaning up %s", mosc) - logger.Infof("OK!\n") - }) - - g.It("[PolarionID:83139][OTP][Skipped:Disconnected] OCB build images in many MCPs at the same time [Disruptive]", func() { - SkipIfCompactOrSNO(oc.AsAdmin()) // This test makes no sense in SNO or compact - - var ( - customMCPNames = "infra" - numCustomPools = 5 - moscList = []*MachineOSConfig{} - mcpList = []*MachineConfigPool{} - wg sync.WaitGroup - ) - - exutil.By("Create custom MCPS") - for i := 0; i < numCustomPools; i++ { - infraMcpName := fmt.Sprintf("%s-%d", customMCPNames, i) - infraMcp, err := CreateCustomMCP(oc.AsAdmin(), infraMcpName, 0) - defer infraMcp.delete() - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating a new custom pool: %s", infraMcpName) - mcpList = append(mcpList, infraMcp) - - } - logger.Infof("OK!\n") - - exutil.By("Checking that all MOSCs were executed properly") - for _, infraMcp := range mcpList { - // MOSCs resources have to use the same name as the MCP - moscName := infraMcp.GetName() - - mosc, err := CreateMachineOSConfigUsingExternalOrInternalRegistry(oc.AsAdmin(), MachineConfigNamespace, moscName, infraMcp.GetName(), nil) - defer mosc.CleanupAndDelete() - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating the MachineOSConfig resource") - moscList = append(moscList, mosc) - - wg.Add(1) - go func() { - defer g.GinkgoRecover() - defer wg.Done() - - ValidateSuccessfulMOSC(mosc, nil) - }() - } - - wg.Wait() - logger.Infof("OK!\n") - - exutil.By("Removing all MOSC resources") - for _, mosc := range moscList { - o.Expect(mosc.CleanupAndDelete()).To(o.Succeed(), "Error cleaning up %s", mosc) - } - logger.Infof("OK!\n") - - exutil.By("Validate that all resources were garbage collected") - for i := 0; i < numCustomPools; i++ { - ValidateMOSCIsGarbageCollected(moscList[i], mcpList[i]) - } - logger.Infof("OK!\n") - - waitForAllMCOPodsReady(oc.AsAdmin(), 10*time.Minute) - logger.Infof("OK!\n") - }) - - g.It("[PolarionID:83755][OTP] In OCL check no new image is applied on node after applying ssh/password/file MC .[Disruptive]", func() { - var ( - mcp = GetCompactCompatiblePool(oc.AsAdmin()) - node = mcp.GetSortedNodesOrFail()[0] - - moscName = mcp.GetName() - mcName = fmt.Sprintf("test-ssh-%s", GetCurrentTestPolarionIDNumber()) - - _, key = GenerateSSHKeyPairOrFail() - user = ign32PaswdUser{Name: "core", SSHAuthorizedKeys: []string{key}} - ) - - exutil.By("Configure OCB functionality for the new worker MCP") - mosc, err := CreateMachineOSConfigUsingExternalOrInternalRegistry(oc.AsAdmin(), MachineConfigNamespace, moscName, mcp.GetName(), nil) - defer DisableOCL(mosc) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating the MachineOSConfig resource") - logger.Infof("Applied MOSC!\n") - - ValidateSuccessfulMOSC(mosc, nil) - logger.Infof("MOSC is applied!\n") - - exutil.By("Get the image that is currently applied on nodes") - initialImage := OrFail[string](node.GetRpmOstreeStatus(false)) - logger.Infof("Initial image: %s", initialImage) - logger.Infof("Got the initial image!\n") - - exutil.By("Create a new MC to deploy new authorized keys") - mc := NewMachineConfig(oc.AsAdmin(), mcName, mcp.GetName()) - mc.parameters = []string{fmt.Sprintf(`PWDUSERS=[%s]`, MarshalOrFail(user))} - mc.skipWaitForMcp = true - - mc.create() - defer mc.DeleteWithWait() - logger.Infof("Created MC!\n") - - exutil.By("Check that the build is triggered with succeed status and not building") - mosb, err := mosc.GetCurrentMachineOSBuild() - logger.Infof("MOSB: %s\n", mosb) - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting MOSB from MOSC") - o.Expect(mosb).To(HaveConditionField("Building", "status", FalseString), "Build is still building") - o.Expect(mosb).To(HaveConditionField("Succeeded", "status", TrueString), "Build didn't succeed") - logger.Infof("Checked that the build does not take place!\n") - - mcp.waitForComplete() - logger.Infof("OK!\n") - - exutil.By("Check that the image is not updated") - o.Expect(OrFail[string](node.GetRpmOstreeStatus(false))).To(o.Equal(initialImage), "Image was updated") - logger.Infof("Image is not updated!\n") - - exutil.By("Check that all expected keys are present and with the right permissions and owners") - currentMc := OrFail[*MachineConfig](mcp.GetConfiguredMachineConfig()) - initialKeys := OrFail[[]string](currentMc.GetAuthorizedKeysByUserAsList("core")) - checkAuthorizedKeyInNode(node, append(initialKeys, key)) - logger.Infof("MC is configured with the expected keys!\n") - - }) - - g.It("[PolarionID:85843][OTP][Skipped:Disconnected] InternalRegistry OCB Verify new nodes boot directly with OCL image without unnecessary reboots [Disruptive]", func() { - SkipIfCompactOrSNO(oc.AsAdmin()) // This test requires scaling, which doesn't make sense in SNO or Compact - skipTestIfWorkersCannotBeScaled(oc.AsAdmin()) // Skip test if worker node cannot be scaled - SkipTestIfCannotUseInternalRegistry(oc.AsAdmin()) // Skip test if cannot use internal registry - - var ( - mcp = GetCompactCompatiblePool(oc.AsAdmin()) - moscName = mcp.GetName() - ) - - exutil.By("Configure OCB functionality for the compatible MCP") - mosc, err := CreateMachineOSConfigUsingInternalRegistry(oc.AsAdmin(), MachineConfigNamespace, moscName, mcp.GetName(), nil, true) - defer DisableOCL(mosc) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating the MachineOSConfig resource") - logger.Infof("OK!\n") - - ValidateNewNodesBootDirectlyWithOCLImage(oc.AsAdmin(), mosc, mcp) - }) - - g.It("[PolarionID:82536][OTP][Skipped:Disconnected] Internal Registry In OCB to check when a image is removed the old build is triggered again and the MC should start updating directly. [Disruptive]", func() { - SkipIfCompactOrSNO(oc.AsAdmin()) - SkipTestIfCannotUseInternalRegistry(oc.AsAdmin()) // This test case requires the internal registry to be enabled - - var ( - infraMcpName = "infra" - mcName = fmt.Sprintf("tc-%s-int-kernelarg", GetCurrentTestPolarionIDNumber()) - kArgs = "test" - ) - - exutil.By("Create custom infra MCP") - infraMcp, err := CreateCustomMCP(oc.AsAdmin(), infraMcpName, 1) - defer DeleteCustomMCP(oc.AsAdmin(), infraMcpName) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating a new custom pool: %s", infraMcpName) - logger.Infof("OK!\n") - - exutil.By("Create MOSC for custom MCP using internal registry") - moscName := infraMcp.GetName() - mosc, err := CreateMachineOSConfigUsingInternalRegistry(oc.AsAdmin(), MachineConfigNamespace, moscName, infraMcp.GetName(), nil, false) - defer DisableOCL(mosc) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating MOSC") - logger.Infof("OK!\n") - - exutil.By("Validate initial MOSC and wait for MOSB-1 to succeed") - ValidateSuccessfulMOSC(mosc, nil) - - mosb1, err := mosc.GetCurrentMachineOSBuild() - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting initial MOSB") - logger.Infof("Initial MOSB created: %s\n", mosb1.GetName()) - - exutil.By("Apply MC with kernel argument to trigger new MOSB") - mc := NewMachineConfig(oc.AsAdmin(), mcName, infraMcp.GetName()) - mc.skipWaitForMcp = true - - defer mc.DeleteWithWait() - - err = mc.Create("-p", "NAME="+mcName, "-p", "POOL="+infraMcp.GetName(), - "-p", fmt.Sprintf(`KERNEL_ARGS=["%s"]`, kArgs)) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating MachineConfig %s", mc.GetName()) - logger.Infof("OK!\n") - - exutil.By("Wait for MOSB-2 to be created and succeed") - checkNewBuildIsTriggered(mosc, mosb1) - mosb2, err := mosc.GetCurrentMachineOSBuild() - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting MOSB-2") - logger.Infof("Second MOSB created: %s\n", mosb2.GetName()) - - exutil.By("Delete MOSB-1 image from internal registry using oc tag -d") - o.Expect(removeImageStream(oc.AsAdmin(), mosb1)).To(o.BeTrue(), "Error deleting imagestream tag for MOSB-1") - logger.Infof("OK!\n") - - // Common verification after MOSB-1 deletion - verifyMOSBRebuildAfterImageDeletion(infraMcp, mosc, mosb1, mosb2, mc, mcName) - }) - - g.It("[PolarionID:79172][OTP] OCB Inherit from global pull secret if baseImagePullSecret field is not specified [Disruptive]", func() { - var ( - infraMcpName = "infra" - ) - - exutil.By("Create custom infra MCP") - // We add no workers to the infra pool, it is not necessary - infraMcp, err := CreateCustomMCP(oc.AsAdmin(), infraMcpName, 0) - defer infraMcp.delete() - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating a new custom pool: %s", infraMcpName) - logger.Infof("OK!\n") - - testContainerFile([]ContainerFile{}, MachineConfigNamespace, infraMcp, nil, true) - }) - - g.It("[PolarionID:83136][OTP][Skipped:Disconnected] Panic Condition for Non-Matching MOSC Resources [Disruptive]", func() { - var ( - infraMcpName = "infra" - // MOSC has to use the same name as the mcp - moscName = infraMcpName - ) - exutil.By("Create New Custom MCP") - defer DeleteCustomMCP(oc.AsAdmin(), infraMcpName) - infraMcp, err := CreateCustomMCP(oc.AsAdmin(), infraMcpName, 1) - o.Expect(err).NotTo(o.HaveOccurred(), "Could not create a new custom MCP") - node := infraMcp.GetNodesOrFail()[0] - logger.Infof("%s", node.GetName()) - logger.Infof("OK!\n") - - exutil.By("Configure OCB functionality for the new infra MCP") - mosc, err := CreateMachineOSConfigUsingExternalOrInternalRegistry(oc.AsAdmin(), MachineConfigNamespace, moscName, infraMcpName, nil) - defer DisableOCL(mosc) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating the MachineOSConfig resource") - logger.Infof("OK!\n") - - exutil.By("Check that a new build has been triggered") - o.Eventually(mosc.GetCurrentMachineOSBuild, "5m", "20s").Should(Exist(), - "No build was created when OCB was enabled") - mosb, err := mosc.GetCurrentMachineOSBuild() - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting MOSB from MOSC") - o.Eventually(mosb.GetJob, "5m", "20s").Should(Exist(), - "No build pod was created when OCB was enabled") - o.Eventually(mosb, "5m", "20s").Should(HaveConditionField("Building", "status", TrueString), - "MachineOSBuild didn't report that the build has begun") - logger.Infof("OK!\n") - - exutil.By("Delete the MCOS and check it is deleted") - o.Expect(mosc.CleanupAndDelete()).To(o.Succeed(), "Error cleaning up %s", mosc) - ValidateMOSCIsGarbageCollected(mosc, infraMcp) - o.Expect(mosb).NotTo(Exist(), "Build is not deleted") - o.Expect(mosc).NotTo(Exist(), "MOSC is not deleted") - logger.Infof("OK!\n") - - exutil.AssertAllPodsToBeReady(oc.AsAdmin(), MachineConfigNamespace) - checkMCCPanic(oc) - }) - - g.It("[PolarionID:78001][OTP][Skipped:Disconnected] The etc-pki-etitlement secret is created automatically for OCB Use custom Containerfile with rhel enablement [Disruptive]", func() { - var ( - entitlementSecret = NewSecret(oc.AsAdmin(), "openshift-config-managed", "etc-pki-entitlement") - containerFileContent = ` - FROM configs AS final - - RUN rm -rf /etc/rhsm-host && \ - rpm-ostree install buildah && \ - ln -s /run/secrets/rhsm /etc/rhsm-host && \ - ostree container commit -` - - checkers = []Checker{ - CommandOutputChecker{ - Command: []string{"rpm", "-q", "buildah"}, - Matcher: o.ContainSubstring("buildah-"), - ErrorMsg: fmt.Sprintf("Buildah package is not installed after the image was deployed"), - Desc: fmt.Sprintf("Check that buildah is installed"), - }, - } - - mcp = GetCompactCompatiblePool(oc.AsAdmin()) - ) - - if !entitlementSecret.Exists() { - g.Skip(fmt.Sprintf("There is no entitlement secret available in this cluster %s. This test case cannot be executed", entitlementSecret)) - } - - testContainerFile([]ContainerFile{{Content: containerFileContent}}, MachineConfigNamespace, mcp, checkers, false) - }) - - g.It("[PolarionID:77498][OTP] OCB Trigger new build when renderedImagePushspec is updated [Disruptive]", func() { - - var ( - infraMcpName = "infra" - // MOSC resources have to use the same names as the MCP - moscName = infraMcpName - ) - - exutil.By("Create custom infra MCP") - // We add no workers to the infra pool, it is not necessary - infraMcp, err := CreateCustomMCP(oc.AsAdmin(), infraMcpName, 0) - defer infraMcp.delete() - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating a new custom pool: %s", infraMcpName) - logger.Infof("OK!\n") - - exutil.By("Configure OCB functionality for the new infra MCP") - mosc, err := CreateMachineOSConfigUsingExternalOrInternalRegistry(oc.AsAdmin(), MachineConfigNamespace, moscName, infraMcpName, nil) - defer mosc.CleanupAndDelete() - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating the MachineOSConfig resource") - logger.Infof("OK!\n") - - ValidateSuccessfulMOSC(mosc, nil) - - exutil.By("Set a new rendered image pull spec") - initialMOSB := OrFail[*MachineOSBuild](mosc.GetCurrentMachineOSBuild()) - initialRIPS := OrFail[string](mosc.GetRenderedImagePushspec()) - o.Expect( - mosc.SetRenderedImagePushspec(strings.ReplaceAll(initialRIPS, "ocb-", "ocb77498-")), - ).NotTo(o.HaveOccurred(), "Error patching %s to set the new renderedImagePullSpec", mosc) - logger.Infof("OK!\n") - - exutil.By("Check that a new build is triggered") - checkNewBuildIsTriggered(mosc, initialMOSB) - logger.Infof("OK!\n") - - exutil.By("Set the original rendered image pull spec") - o.Expect( - mosc.SetRenderedImagePushspec(initialRIPS), - ).NotTo(o.HaveOccurred(), "Error patching %s to set the new renderedImagePullSpec", mosc) - logger.Infof("OK!\n") - - exutil.By("Check that the initial build is reused") - var currentMOSB *MachineOSBuild - o.Eventually(func() (string, error) { - currentMOSB, err = mosc.GetCurrentMachineOSBuild() - if err != nil || currentMOSB == nil { - return "", err - } - return currentMOSB.GetName(), nil - }, "5m", "20s").Should(o.Equal(initialMOSB.GetName()), - "When the containerfiles were removed and initial MOSC configuration was restored, the initial MOSB was not used") - - logger.Infof("OK!\n") - }) - - g.It("[PolarionID:77497][OTP] OCB Trigger new build when Containerfile is updated [Disruptive]", func() { - - var ( - infraMcpName = "infra" - // MOSC resources have to use the same names as the MCP - moscName = infraMcpName - containerFile = ContainerFile{Content: "RUN touch /etc/test-add-containerfile" + "\n" + ExpirationDockerfileLabel} - containerFileMod = ContainerFile{Content: "RUN touch /etc/test-modified-containerfile" + "\n" + ExpirationDockerfileLabel} - - // We need to test a first MOSC without any container file, so we cannot add the expiration label in the first MOSC - expireImage = false - // We don't configure the pull secret in the MOSC, since it is optional - defaultPullSecret = true - ) - - exutil.By("Create custom infra MCP") - // We add no workers to the infra pool, it is not necessary - infraMcp, err := CreateCustomMCP(oc.AsAdmin(), infraMcpName, 0) - defer infraMcp.delete() - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating a new custom pool: %s", infraMcpName) - logger.Infof("OK!\n") - - exutil.By("Configure OCB functionality for the new infra MCP") - mosc, err := createMachineOSConfigUsingExternalOrInternalRegistry(oc.AsAdmin(), MachineConfigNamespace, moscName, infraMcpName, nil, defaultPullSecret, expireImage) - - defer mosc.CleanupAndDelete() - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating the MachineOSConfig resource") - logger.Infof("OK!\n") - - ValidateSuccessfulMOSC(mosc, nil) - - exutil.By("Add new container file") - initialMOSB := OrFail[*MachineOSBuild](mosc.GetCurrentMachineOSBuild()) - - o.Expect( - mosc.SetContainerfiles([]ContainerFile{containerFile}), - ).NotTo(o.HaveOccurred(), "Error patching %s to add a container file", mosc) - logger.Infof("OK!\n") - - exutil.By("Check that a new build is triggered when a containerfile is added") - checkNewBuildIsTriggered(mosc, initialMOSB) - logger.Infof("OK!\n") - - exutil.By("Modify the container file") - currentMOSB := OrFail[*MachineOSBuild](mosc.GetCurrentMachineOSBuild()) - - o.Expect( - mosc.SetContainerfiles([]ContainerFile{containerFileMod}), - ).NotTo(o.HaveOccurred(), "Error patching %s to modify an existing container file", mosc) - logger.Infof("OK!\n") - - exutil.By("Check that a new build is triggered when a containerfile is modified") - checkNewBuildIsTriggered(mosc, currentMOSB) - logger.Infof("OK!\n") - - exutil.By("Remove the container files") - o.Expect( - mosc.RemoveContainerfiles(), - ).NotTo(o.HaveOccurred(), "Error patching %s to remove the configured container files", mosc) - logger.Infof("OK!\n") - - exutil.By("Check that the initial build is reused") - o.Eventually(func() (string, error) { - currentMOSB, err = mosc.GetCurrentMachineOSBuild() - if err != nil || currentMOSB == nil { - return "", err - } - return currentMOSB.GetName(), nil - }, "5m", "20s").Should(o.Equal(initialMOSB.GetName()), - "When the containerfiles were removed and initial MOSC configuration was restored, the initial MOSB was not used") - - logger.Infof("OK!\n") - }) - - g.It("[PolarionID:77576][OTP] In OCB. Create a new MC while a build is running [Disruptive]", func() { - - var ( - mcp = GetCompactCompatiblePool(oc.AsAdmin()) - node = mcp.GetSortedNodesOrFail()[0] - moscName = mcp.GetName() - - kArgs = "test" - mcName = "tc-77576-testkargs" - mc = NewMachineConfig(oc.AsAdmin(), mcName, mcp.GetName()) - ) - - exutil.By("Configure OCB functionality for the new worker MCP") - mosc, err := CreateMachineOSConfigUsingExternalOrInternalRegistry(oc.AsAdmin(), MachineConfigNamespace, moscName, mcp.GetName(), nil) - defer DisableOCL(mosc) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating the MachineOSConfig resource") - logger.Infof("OK!\n") - - exutil.By("Check that a new build has been triggered and is building") - var mosb *MachineOSBuild - o.Eventually(func() (*MachineOSBuild, error) { - var err error - mosb, err = mosc.GetCurrentMachineOSBuild() - return mosb, err - }, "5m", "20s").Should(Exist(), - "No build was created when OCB was enabled") - o.Eventually(mosb.GetJob, "5m", "20s").Should(Exist(), - "No build job was created when OCB was enabled") - o.Eventually(mosb, "5m", "20s").Should(HaveConditionField("Building", "status", TrueString), - "MachineOSBuild didn't report that the build has begun") - logger.Infof("OK!\n") - - exutil.By("Create a MC to trigger a new build") - defer mc.DeleteWithWait() - err = mc.Create("-p", "NAME="+mcName, "-p", "POOL="+mcp.GetName(), "-p", fmt.Sprintf(`KERNEL_ARGS=["%s"]`, kArgs)) - o.Expect(err).NotTo(o.HaveOccurred()) - logger.Infof("OK!\n") - - exutil.By("Check that a new build is triggered and the old build is removed") - checkNewBuildIsTriggered(mosc, mosb) - o.Eventually(mosb, "2m", "20s").ShouldNot(Exist(), "The old MOSB %s was not deleted", mosb) - logger.Infof("OK!\n") - - exutil.By("Wait for the configuration to be applied") - mcp.waitForComplete() - logger.Infof("OK!\n") - - exutil.By("Check that the MC was applied") - o.Expect(node.IsKernelArgEnabled(kArgs)).To(o.BeTrue(), - "Kernel argument %s was not enabled in the node %s", kArgs, node.GetName()) - - logger.Infof("OK!\n") - - exutil.By("Remove the MachineOSConfig resource") - o.Expect(DisableOCL(mosc)).To(o.Succeed(), "Error cleaning up %s", mosc) - logger.Infof("OK!\n") - }) - - g.It("[PolarionID:77977][OTP][Skipped:Disconnected] Install extension after OCB is enabled [Disruptive]", func() { - - var ( - mcp = GetCompactCompatiblePool(oc.AsAdmin()) - moscName = mcp.GetName() // MOSC resources have to use the same name as the MCP - node = mcp.GetSortedNodesOrFail()[0] - mcName = "test-install-extension-" + GetCurrentTestPolarionIDNumber() - applicableExtensions, _ = GetAllApplicableExtensionsToMCPOrFail(mcp) - ) - - exutil.By("Configure OCB functionality for the new worker MCP") - mosc, err := CreateMachineOSConfigUsingExternalOrInternalRegistry(oc.AsAdmin(), MachineConfigNamespace, moscName, mcp.GetName(), nil) - defer DisableOCL(mosc) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating the MachineOSConfig resource") - logger.Infof("OK!\n") - - ValidateSuccessfulMOSC(mosc, nil) - - exutil.By("Create a MC") - mc := NewMachineConfig(oc.AsAdmin(), mcName, mcp.GetName()) - defer mc.DeleteWithWait() - mc.parameters = []string{fmt.Sprintf(`EXTENSIONS=%s`, string(MarshalOrFail(applicableExtensions)))} - mc.create() - logger.Infof("OK!\n") - - exutil.By("Wait for the configuration to be applied") - mcp.waitForComplete() - logger.Infof("OK!\n") - - CheckExtensions(node, applicableExtensions) - - exutil.By("Delete a MC.") - mc.DeleteWithWait() - logger.Infof("OK!\n") - - exutil.By("Remove the MachineOSConfig resource") - o.Expect(DisableOCL(mosc)).To(o.Succeed(), "Error cleaning up %s", mosc) - ValidateMOSCIsGarbageCollected(mosc, mcp) - logger.Infof("OK!\n") - }) - - g.It("[PolarionID:78196][OTP][Skipped:Disconnected] Verify for etc-pki-etitlement secret is removed for OCB rhel enablement [Disruptive]", func() { - - var ( - entitlementSecret = NewSecret(oc.AsAdmin(), "openshift-config-managed", "etc-pki-entitlement") - containerFileContent = ` - FROM configs AS final - RUN rm -rf /etc/rhsm-host && \ - rpm-ostree install buildah && \ - ln -s /run/secrets/rhsm /etc/rhsm-host && \ - ostree container commit - ` - - mcp = GetCompactCompatiblePool(oc.AsAdmin()) - // MOSC resources have to use the same name as the MCP - moscName = mcp.GetName() - ) - if !entitlementSecret.Exists() { - g.Skip(fmt.Sprintf("There is no entitlement secret available in this cluster %s. This test case cannot be executed", entitlementSecret)) - } - - exutil.By("Copy the entitlement secret in MCO namespace") - mcoEntitlementSecret, err := CloneResource(entitlementSecret, "etc-pki-entitlement", MachineConfigNamespace, nil) - defer mcoEntitlementSecret.Delete() - o.Expect(err).NotTo(o.HaveOccurred(), "Error copying %s to the %s namespace", mcoEntitlementSecret, MachineConfigNamespace) - logger.Infof("OK!\n") - - exutil.By("Delete the entitlement secret in the openshift-config-managed namespace") - defer func() { - exutil.By("Recover the entitlement secret in the openshift-config-managed namespace") - recoverSecret, err := CloneResource(mcoEntitlementSecret, "etc-pki-entitlement", "openshift-config-managed", nil) - o.Expect(err).NotTo(o.HaveOccurred(), "Error copying %s to the openshift-config-managed namespace", entitlementSecret) - o.Expect(recoverSecret).To(Exist(), "Unable to recover the entitlement secret in openshift-config-managed namespace") - }() - - entitlementSecret.Delete() - logger.Infof("OK!\n") - - exutil.By("Create the MOSC") - mosc, err := CreateMachineOSConfigUsingExternalOrInternalRegistry(oc.AsAdmin(), MachineConfigNamespace, moscName, mcp.GetName(), []ContainerFile{{Content: containerFileContent}}) - defer DisableOCL(mosc) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating the MachineOSConfig resource") - logger.Infof("OK!\n") - - exutil.By("Check that a new build has been triggered") - o.Eventually(mosc.GetCurrentMachineOSBuild, "5m", "20s").Should(Exist(), - "No build was created when OCB was enabled") - mosb, err := mosc.GetCurrentMachineOSBuild() - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting MOSB from MOSC") - o.Eventually(mosb, "5m", "20s").Should(HaveConditionField("Building", "status", TrueString), - "MachineOSBuild didn't report that the build has begun") - logger.Infof("OK!\n") - - exutil.By("Verify the error is produced in buildPod") - exutil.AssertAllNonJobPodsToBeReadyWithPollerParams(mosc.GetOC(), MachineConfigNamespace, 10*time.Second, 10*time.Minute) - logger.Infof("OK!\n") - - job, err := mosb.GetJob() - o.Expect(err).NotTo(o.HaveOccurred()) - // Currently this kind of resources are leaked. Until the leak is fixed we need to make sure that this job is removed - // because its pods are in "Error" status and there are other test cases checking that no pod is reporting any error. - // TODO: remove this once the leak is fixed - defer job.Delete() - logger.Infof("OK!\n") - - o.Eventually(func() (string, error) { - return job.Logs("-c", "image-build") - }, "5m", "10s").Should(o.ContainSubstring("Found 0 entitlement certificates"), "Error getting the logs") - o.Eventually(job, "15m", "20s").Should(HaveConditionField("Failed", "status", TrueString), "Job didn't fail") - logger.Infof("OK!\n") - - exutil.By("Remove the MachineOSConfig resource") - o.Expect(DisableOCL(mosc)).To(o.Succeed(), "Error cleaning up %s", mosc) - ValidateMOSCIsGarbageCollected(mosc, mcp) - logger.Infof("OK!\n") - }) - - g.It("[PolarionID:88801][OTP][Skipped:Disconnected] ExternalRegistry OCB Verify new nodes boot directly with OCL image without unnecessary reboots [Disruptive]", g.Label("Exclude: excluded until OCPBUGS-99751 is fixed"), func() { - SkipIfCompactOrSNO(oc.AsAdmin()) // This test requires scaling, which doesn't make sense in SNO or Compact - skipTestIfWorkersCannotBeScaled(oc.AsAdmin()) // Skip test if worker node cannot be scaled - - var ( - mcp = GetCompactCompatiblePool(oc.AsAdmin()) - moscName = mcp.GetName() - ) - - exutil.By("Configure OCB functionality using external registry (Quay)") - mosc, err := CreateMachineOSConfigUsingExternalRegistry(oc.AsAdmin(), moscName, mcp.GetName(), nil, false, false) - defer DisableOCL(mosc) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating the MachineOSConfig resource") - logger.Infof("OK!\n") - - ValidateNewNodesBootDirectlyWithOCLImage(oc.AsAdmin(), mosc, mcp) - }) - - g.It("[PolarionID:85980][OTP][Skipped:Disconnected] Check when MOSB is degraded MCP should be degraded too with right fields updated. [Disruptive]", func() { - var ( - infraMcpName = "infra" - moscName = infraMcpName - // Intentionally uses 'apt' on Alpine (which uses 'apk'), causing build to fail - incorrectContainerFile = "FROM alpine:3.18\nRUN apt update && apt install -y cowsay\n" - correctContainerFile = "FROM configs AS final\nRUN echo \"hello\" > /etc/test.txt\n" - expectedDegradedMessage = "Failed to build OS image" - ) - - exutil.By("Create custom infra MCP") - infraMcp, err := CreateCustomMCP(oc.AsAdmin(), infraMcpName, 1) - defer DeleteCustomMCP(oc.AsAdmin(), infraMcpName) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating custom MCP: %s", infraMcpName) - node := infraMcp.GetNodesOrFail()[0] - logger.Infof("Infra node: %s\n", node.GetName()) - logger.Infof("OK!\n") - - exutil.By("Create MOSC with incorrect containerfile") - mosc, err := CreateMachineOSConfigUsingExternalOrInternalRegistry(oc.AsAdmin(), MachineConfigNamespace, moscName, infraMcpName, - []ContainerFile{{Content: incorrectContainerFile}}) - defer mosc.CleanupAndDelete() - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating MOSC with incorrect containerfile") - logger.Infof("OK!\n") - - exutil.By("Verify MOSB is created and fails") - o.Eventually(mosc.GetCurrentMachineOSBuild, "5m", "20s").Should(Exist(), - "No MOSB was created for the incorrect containerfile") - failedMOSB, err := mosc.GetCurrentMachineOSBuild() - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting MOSB from MOSC") - logger.Infof("MOSB created: %s\n", failedMOSB.GetName()) - o.Eventually(failedMOSB, "20m", "20s").Should(HaveConditionField("Failed", "status", TrueString), - "MOSB should fail due to incorrect containerfile") - logger.Infof("MOSB failed as expected\n") - - exutil.By("Verify MCP is degraded with ImageBuildDegraded condition") - o.Eventually(infraMcp, "5m", "20s").Should(BeDegraded(), "MCP should be degraded when build fails") - o.Eventually(infraMcp, "5m", "20s").Should(HaveConditionField("ImageBuildDegraded", "status", TrueString), - "MCP ImageBuildDegraded should be True when build fails") - o.Eventually(infraMcp, "5m", "20s").Should(HaveConditionField("ImageBuildDegraded", "message", o.ContainSubstring(expectedDegradedMessage)), - "MCP degradation message should indicate OS image build failure") - o.Eventually(infraMcp, "5m", "20s").Should(HaveConditionField("Degraded", "message", o.ContainSubstring("Custom OS image build failed")), - "MCP Degraded message should indicate custom OS image build failed") - logger.Infof("OK!\n") - - exutil.By("Fix the MOSC by updating containerfile") - o.Expect(mosc.SetContainerfiles([]ContainerFile{{Content: correctContainerFile}})).NotTo(o.HaveOccurred(), - "Error updating MOSC with correct containerfile") - logger.Infof("OK!\n") - - exutil.By("Verify new MOSB is created and succeeds") - checkNewBuildIsTriggered(mosc, failedMOSB) - newMOSB, err := mosc.GetCurrentMachineOSBuild() - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting new MOSB") - logger.Infof("New MOSB created: %s\n", newMOSB.GetName()) - - exutil.By("Verify old failed MOSB is deleted") - o.Eventually(failedMOSB, "5m", "20s").ShouldNot(Exist(), - "Old failed MOSB should be garbage collected: %s", failedMOSB.GetName()) - logger.Infof("OK!\n") - - exutil.By("Verify MCP recovers and is no longer degraded") - // Check both ImageBuildDegraded (specific condition) and Degraded (overall status) - // to ensure complete recovery from the failed build state - o.Eventually(infraMcp, "5m", "20s").Should(HaveConditionField("ImageBuildDegraded", "status", FalseString), - "MCP ImageBuildDegraded condition should be False after recovery") - o.Eventually(infraMcp, "5m", "20s").ShouldNot(BeDegraded(), - "MCP Degraded condition should be False after recovery") - o.Eventually(infraMcp, "5m", "20s").Should(HaveConditionField("Updating", "status", TrueString), - "MCP should be updating after successful build") - logger.Infof("OK!\n") - - exutil.By("Wait for MCP to complete update") - infraMcp.waitForComplete() - logger.Infof("OK!\n") - - exutil.By("Verify image is applied to node") - currentImagePullSpec := OrFail[string](mosc.GetStatusCurrentImagePullSpec()) - o.Expect(node.GetCurrentBootOSImage()).To(o.Equal(currentImagePullSpec), - "Node should be using the correct OCL image after recovery") - logger.Infof("Node is using image: %s\n", currentImagePullSpec) - logger.Infof("OK!\n") - - exutil.By("Verify file created in containerfile exists on node") - output, err := node.DebugNodeWithChroot("cat", "/etc/test.txt") - o.Expect(err).NotTo(o.HaveOccurred(), "Error reading test file from node") - o.Expect(output).To(o.ContainSubstring("hello"), - "Test file content should match what was created in containerfile") - logger.Infof("OK!\n") - }) - - g.It("[PolarionID:87176][OTP][Skipped:Disconnected] External Registry In OCB to check when a image is removed the old build is triggered again and the MC should start updating directly. [Disruptive]", func() { - SkipIfCompactOrSNO(oc.AsAdmin()) - - var ( - infraMcpName = "infra" - mcName = fmt.Sprintf("tc-%s-ext-kernelarg", GetCurrentTestPolarionIDNumber()) - kArgs = "test" - containerFileNoExpiration = "FROM configs AS final\nLABEL maintainer=\"mco@example.com\"\n" - ) - - exutil.By("Create custom infra MCP") - infraMcp, err := CreateCustomMCP(oc.AsAdmin(), infraMcpName, 1) - defer DeleteCustomMCP(oc.AsAdmin(), infraMcpName) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating a new custom pool: %s", infraMcpName) - logger.Infof("OK!\n") - - exutil.By("Create MOSC for custom MCP using external registry") - moscName := infraMcp.GetName() - mosc, err := CreateMachineOSConfigUsingExternalRegistry(oc.AsAdmin(), moscName, infraMcp.GetName(), - []ContainerFile{{Content: containerFileNoExpiration}}, false, false) - defer DisableOCL(mosc) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating MOSC") - logger.Infof("OK!\n") - - exutil.By("Validate initial MOSC and wait for MOSB-1 to succeed") - ValidateSuccessfulMOSC(mosc, nil) - - mosb1, err := mosc.GetCurrentMachineOSBuild() - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting initial MOSB") - logger.Infof("Initial MOSB created: %s\n", mosb1.GetName()) - - exutil.By("Apply MC with kernel argument to trigger new MOSB") - mc := NewMachineConfig(oc.AsAdmin(), mcName, infraMcp.GetName()) - mc.skipWaitForMcp = true - - defer mc.DeleteWithWait() - - err = mc.Create("-p", "NAME="+mcName, "-p", "POOL="+infraMcp.GetName(), - "-p", fmt.Sprintf(`KERNEL_ARGS=["%s"]`, kArgs)) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating MachineConfig %s", mc.GetName()) - logger.Infof("OK!\n") - - exutil.By("Wait for MOSB-2 to be created and succeed") - checkNewBuildIsTriggered(mosc, mosb1) - mosb2, err := mosc.GetCurrentMachineOSBuild() - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting MOSB-2") - logger.Infof("Second MOSB created: %s\n", mosb2.GetName()) - - exutil.By("Wait for MCP to complete update") - infraMcp.waitForComplete() - logger.Infof("OK!\n") - - exutil.By("Delete MOSB-1 image from Quay using automated skopeo deletion") - o.Expect(removeQuayImageUsingSkopeo(oc.AsAdmin(), mosb1, infraMcp)).To(o.BeTrue(), "Error deleting Quay image for MOSB-1") - logger.Infof("OK!\n") - - // Common verification after MOSB-1 deletion - verifyMOSBRebuildAfterImageDeletion(infraMcp, mosc, mosb1, mosb2, mc, mcName) - }) - -}) - -func checkNewBuildIsTriggered(mosc *MachineOSConfig, currentMOSB *MachineOSBuild) { - var ( - newMOSB *MachineOSBuild - err error - ) - logger.Infof("Current mosb: %s", currentMOSB) - o.Eventually(func() (string, error) { - newMOSB, err = mosc.GetCurrentMachineOSBuild() - if err != nil || newMOSB == nil { - return "", err - } - return newMOSB.GetName(), nil - }, "5m", "20s").ShouldNot(o.Equal(currentMOSB.GetName()), - "A new MOSB should be created after the new rendered image pull spec is configured") - - logger.Infof("New mosb: %s", newMOSB) - - o.Eventually(newMOSB, "5m", "20s").Should(HaveConditionField("Building", "status", TrueString), - "MachineOSBuild didn't report that the build has begun") - - o.Eventually(newMOSB, "20m", "20s").Should(HaveConditionField("Building", "status", FalseString), "Build was not finished") - o.Eventually(newMOSB, "10m", "20s").Should(HaveConditionField("Succeeded", "status", TrueString), "Build didn't succeed") - o.Eventually(newMOSB, "2m", "20s").Should(HaveConditionField("Interrupted", "status", FalseString), "Build was interrupted") - o.Eventually(newMOSB, "2m", "20s").Should(HaveConditionField("Failed", "status", FalseString), "Build was failed") -} - -func verifyNewNodeBootedWithOCLImage(newNode *Node, oclImage string) { - exutil.By("Verify the new node boots directly with the OCL image") - currentImage, err := newNode.GetCurrentBootOSImage() - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting current boot OS image from node %s", newNode.GetName()) - o.Expect(currentImage).To(o.Equal(oclImage), "New node %s is not using the OCL image. Current: %s, Expected: %s", newNode.GetName(), currentImage, oclImage) - logger.Infof("OK!\n") - - exutil.By("Verify /etc/machine-config-daemon/currentimage matches the OCL image") - currentImageFile, err := newNode.DebugNodeWithChroot("cat", "/etc/machine-config-daemon/currentimage") - o.Expect(err).NotTo(o.HaveOccurred(), "Error reading currentimage file from node %s", newNode.GetName()) - o.Expect(currentImageFile).To(o.ContainSubstring(oclImage), "currentimage file does not contain the OCL image") - logger.Infof("OK!\n") - - exutil.By("Verify rpm-ostree status shows the OCL image") - rpmOstreeStatus, err := newNode.DebugNodeWithChroot("rpm-ostree", "status") - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting rpm-ostree status from node %s", newNode.GetName()) - o.Expect(rpmOstreeStatus).To(o.ContainSubstring(oclImage), "rpm-ostree status does not show the OCL image") - logger.Infof("OK!\n") - - exutil.By("Verify no unnecessary reboots occurred - desiredImage file should not exist") - desiredImageFile, err := newNode.DebugNodeWithChroot("cat", "/etc/machine-config-daemon/desiredImage") - o.Expect(err).To(o.HaveOccurred(), "desiredImage file should not exist when node boots with correct OCL image") - o.Expect(desiredImageFile).Should(o.ContainSubstring("No such file or directory"), - "Expected desiredImage file to not exist, indicating no OS upgrade was needed") - logger.Infof("OK!\n") -} - -func ValidateNewNodesBootDirectlyWithOCLImage(oc *exutil.CLI, mosc *MachineOSConfig, mcp *MachineConfigPool) { - ValidateSuccessfulMOSC(mosc, nil) - - exutil.By("Get the OCL image") - oclImage := OrFail[string](mosc.GetStatusCurrentImagePullSpec()) - logger.Infof("OCL image: %s\n", oclImage) - - isInternalRegistry := OrFail[bool](mosc.IsUsingInternalRegistry()) - - exutil.By("Check able to scale the node from existing Machineset") - msl, err := NewMachineSetList(oc.AsAdmin(), MachineAPINamespace).GetAll() - o.Expect(err).NotTo(o.HaveOccurred(), "Get machinesets failed") - o.Expect(msl).ShouldNot(o.BeEmpty(), "Machineset list is empty") - existingMS := msl[0] - - o.Expect(existingMS.AddToScale(1)).NotTo(o.HaveOccurred()) - - defer func() { - exutil.By("Scale down the node from existing Machineset") - existingMS.AddToScale(-1) - mcp.waitForComplete() - }() - - exutil.By("Create duplicate machineset and scale new node") - machineset := OrFail[*MachineSet](GetScalableMachineSet(oc.AsAdmin())) - duplicateMSName := machineset.GetName() + "-ocl" - duplicateMS, err := machineset.Duplicate(duplicateMSName) - o.Expect(err).NotTo(o.HaveOccurred()) - - defer func() { - if duplicateMS.Exists() { - logger.Infof("Deleting duplicate machineset %s and scale down the node", duplicateMS.GetName()) - o.Expect(duplicateMS.ScaleTo(0)).To(o.Succeed()) - o.Expect(duplicateMS.WaitUntilReady("10m")).To(o.Succeed()) - o.Expect(duplicateMS.Delete()).To(o.Succeed()) - } - }() - o.Expect(duplicateMS.ScaleTo(1)).To(o.Succeed()) - - exutil.By("Wait for both existing new node added and for duplicate new node added to get ready") - - o.Eventually(func(gm o.Gomega) { - gm.Expect(existingMS.GetIsReady()).To(o.BeTrue(), "MachineSet %s is not ready", existingMS.GetName()) - gm.Expect(duplicateMS.GetIsReady()).To(o.BeTrue(), "MachineSet %s is not ready", duplicateMS.GetName()) - }, "30m", "2m").Should(o.Succeed(), "MachineSets are not ready") - - if isInternalRegistry { - logger.Infof("Internal registry detected, waiting for MCP to complete (requires 2 reboots)") - mcp.waitForComplete() - } - logger.Infof("OK!\n") - - exutil.By("Get the new node created by the machine set") - - existingMSNodes, nErr := existingMS.GetNodes() - o.Expect(nErr).NotTo(o.HaveOccurred(), "Error getting the nodes created by MachineSet %s", existingMS.GetName()) - existingNode := existingMSNodes[len(existingMSNodes)-1] - logger.Infof("Existing node: %s\n", existingNode.GetName()) - logger.Infof("OK!\n") - - duplicateMSNodes, nErr := duplicateMS.GetNodes() - o.Expect(nErr).NotTo(o.HaveOccurred(), "Error getting the nodes created by MachineSet %s", duplicateMS.GetName()) - duplicateMSNode := duplicateMSNodes[len(duplicateMSNodes)-1] - logger.Infof("Duplicate node: %s\n", duplicateMSNode.GetName()) - logger.Infof("OK!\n") - - verifyNewNodeBootedWithOCLImage(duplicateMSNode, oclImage) - verifyNewNodeBootedWithOCLImage(existingNode, oclImage) - - exutil.By("Wait for MCP to complete before scaling down nodes") - mcp.waitForComplete() - logger.Infof("OK!\n") -} - -func removeImageStream(oc *exutil.CLI, mosb *MachineOSBuild) bool { - mosc, err := mosb.GetMachineOSConfig() - if err != nil { - logger.Errorf("Error getting MOSC from MOSB: %s", err) - return false - } - - pushSpec, err := mosc.GetRenderedImagePushspec() - if err != nil { - logger.Errorf("Error getting renderedImagePushspec from MOSC: %s", err) - return false - } - - parts := strings.Split(pushSpec, "/") - if len(parts) < 3 { - logger.Errorf("Invalid push spec format: %s", pushSpec) - return false - } - imageNameWithTag := parts[len(parts)-1] - imageName := strings.Split(imageNameWithTag, ":")[0] - - logger.Infof("Deleting imagestream tag: %s:%s", imageName, mosb.GetName()) - imagestream, err := oc.AsAdmin().WithoutNamespace().Run("tag").Args("-d", imageName+":"+mosb.GetName(), "-n", MachineConfigNamespace).Output() - - if err != nil { - logger.Errorf("Error deleting imagestream: %s", imagestream) - return false - } - return true -} - -func removeQuayImageUsingSkopeo(oc *exutil.CLI, mosb *MachineOSBuild, mcp *MachineConfigPool) bool { - mosc, err := mosb.GetMachineOSConfig() - if err != nil { - logger.Errorf("Error getting MOSC from MOSB: %s", err) - return false - } - - digestedImage, err := mosb.GetStatusDigestedImagePullSpec() - if err != nil { - logger.Errorf("Error getting digested image from MOSB: %s", err) - return false - } - - logger.Infof("Attempting to delete Quay image: %s", digestedImage) - - pushSecretName, err := mosc.Get(`{.spec.renderedImagePushSecret.name}`) - if err != nil || pushSecretName == "" { - logger.Errorf("Error getting push secret name from MOSC: %s", err) - return false - } - - logger.Infof("Using push secret for deletion: %s", pushSecretName) - - pushSecret := NewSecret(oc, MachineConfigNamespace, pushSecretName) - secretDir, err := pushSecret.Extract() - if err != nil { - logger.Errorf("Error extracting push secret: %s", err) - return false - } - defer func() { - logger.Infof("Cleaning up local secret directory: %s", secretDir) - os.RemoveAll(secretDir) - }() - logger.Infof("Push secret extracted to local directory: %s", secretDir) - - localAuthFile := filepath.Join(secretDir, ".dockerconfigjson") - - nodes, err := mcp.GetNodes() - if err != nil || len(nodes) == 0 { - logger.Errorf("Error getting nodes from MCP: %s", err) - return false - } - node := nodes[0] - - logger.Infof("Running skopeo delete on node: %s", node.GetName()) - - tmpAuthFile := "/tmp/skopeo-delete-auth-" + exutil.GetRandomString() + ".json" - - defer func() { - logger.Infof("Cleaning up temporary auth file on node") - node.DebugNodeWithChroot("rm", "-f", tmpAuthFile) - }() - - err = node.CopyFromLocal(localAuthFile, tmpAuthFile) - if err != nil { - logger.Errorf("Error copying auth file to node: %s", err) - return false - } - - skopeoCommand := fmt.Sprintf("skopeo delete --authfile %s docker://%s", tmpAuthFile, digestedImage) - fullCommand := fmt.Sprintf("set -a; source /etc/mco/proxy.env; %s", skopeoCommand) - - logger.Infof("Executing command on node: %s", skopeoCommand) - - stdout, stderr, err := node.DebugNodeWithChrootStd("sh", "-c", fullCommand) - if err != nil { - logger.Errorf("Error deleting Quay image via skopeo on node. Stdout: %s, Stderr: %s, Error: %s", stdout, stderr, err) - return false - } - - logger.Infof("Successfully deleted Quay image: %s", digestedImage) - logger.Infof("Skopeo output: %s", stdout) - return true -} - -func verifyMOSBRebuildAfterImageDeletion(mcp *MachineConfigPool, mosc *MachineOSConfig, mosb1, mosb2 *MachineOSBuild, mc *MachineConfig, mcName string) { - exutil.By("Delete MC") - err := mc.Delete() - o.Expect(err).NotTo(o.HaveOccurred(), "Error deleting MC") - logger.Infof("OK!\n") - - exutil.By("Verify MOSB-1 is re-triggered (starts building again)") - o.Eventually(mosb1, "5m", "20s").Should(HaveConditionField("Building", "status", TrueString), - "MOSB-1 should be re-triggered and start building again after image deletion") - logger.Infof("MOSB-1 is re-building\n") - - o.Eventually(mosb1, "35m", "20s").Should(HaveConditionField("Building", "status", FalseString), - "MOSB-1 rebuild should complete") - o.Eventually(mosb1, "10m", "20s").Should(HaveConditionField("Succeeded", "status", TrueString), - "MOSB-1 rebuild should succeed") - logger.Infof("OK!\n") - - exutil.By("Wait for MCP to complete update") - mcp.waitForComplete() - logger.Infof("OK!\n") - - exutil.By("Re-apply MC") - mc.skipWaitForMcp = true - defer mc.DeleteWithWait() - err = mc.Create("-p", "NAME="+mcName, "-p", "POOL="+mcp.GetName(), - "-p", fmt.Sprintf(`KERNEL_ARGS=["%s"]`, "test")) - o.Expect(err).NotTo(o.HaveOccurred(), "Error creating MachineConfig %s", mc.GetName()) - logger.Infof("OK!\n") - - exutil.By("Verify MOSB-2 is NOT triggered again") - o.Consistently(func() bool { - buildingStatus, err := mosb2.Get(`{.status.conditions[?(@.type=="Building")].status}`) - if err != nil || buildingStatus == "" { - return false - } - return buildingStatus == TrueString - }, "2m", "10s").Should(o.BeFalse(), "MOSB-2 should NOT start building again") - logger.Infof("OK!\n") - - exutil.By("Verify MCP starts updating instead of triggering a new/old MOSB") - o.Eventually(mcp, "5m", "20s").Should(HaveConditionField("Updating", "status", TrueString), - "MCP should start updating instead of triggering a new/old MOSB") - logger.Infof("OK!\n") - - exutil.By("Wait for MCP to complete update") - mcp.waitForComplete() - logger.Infof("OK!\n") - - exutil.By("Verify no new MOSB-3 was created during MCP update") - currentMosb, err := mosc.GetCurrentMachineOSBuild() - logger.Infof("Current MOSB: %s\n", currentMosb.GetName()) - logger.Infof("MOSB-1: %s\n", mosb1.GetName()) - logger.Infof("MOSB-2: %s\n", mosb2.GetName()) - o.Expect(err).NotTo(o.HaveOccurred(), "Error getting current MOSB") - o.Expect(currentMosb.GetName()).To(o.Or(o.Equal(mosb1.GetName()), o.Equal(mosb2.GetName())), - "Current MOSB should be either MOSB-1 or MOSB-2, no new MOSB-3 should be triggered") - logger.Infof("OK!\n") -} diff --git a/test/extended-priv/mco_scale.go b/test/extended-priv/mco_scale.go index 171e3e2af6..51e895e400 100644 --- a/test/extended-priv/mco_scale.go +++ b/test/extended-priv/mco_scale.go @@ -472,10 +472,23 @@ func SimpleScaleUPTest(oc *exutil.CLI, mcp *MachineConfigPool, imageVersion, ign logger.Infof("OK!\n") } - defer SafeCleanup(func() { + defer func() { + logger.Infof("Start TC defer block") newMs := NewMachineSet(oc.AsAdmin(), MachineAPINamespace, newMsName) - removeClonedMachineSet(newMs, mcp, initialNumWorkers) - }) + errors := o.InterceptGomegaFailures(func() { removeClonedMachineSet(newMs, mcp, initialNumWorkers) }) // We don't want gomega to fail and stop the deferred cleanup process + if len(errors) != 0 { + logger.Infof("There were errors restoring the original MachineSet resources in the cluster") + for _, e := range errors { + logger.Errorf(e) + } + } + + // We don't want the test to pass if there were errors while restoring the initial state + o.Expect(len(errors)).To(o.BeZero(), + "There were %d errors while recovering the cluster's initial state", len(errors)) + + logger.Infof("End TC defer block") + }() logger.Infof("Create a new MachineSet using the right base image") allMs, err := NewMachineSetList(oc.AsAdmin(), MachineAPINamespace).GetAll() diff --git a/test/extended-priv/node.go b/test/extended-priv/node.go index 65a7e74e8c..42d7e20c4b 100644 --- a/test/extended-priv/node.go +++ b/test/extended-priv/node.go @@ -1457,12 +1457,24 @@ func FixRebootInNode(node *Node) error { // BreakRebaseInNode breaks the rpm-ostree rebase process in a node func BreakRebaseInNode(node *Node) error { logger.Infof("Breaking rpm-ostree rebase process in node %s", node.GetName()) - return ReplaceRpmOstree(node, generateTemplateAbsolutePath("rpm-ostree-force-pivot-error.sh")) + brokenRpmOstree := generateTemplateAbsolutePath("rpm-ostree-force-pivot-error.sh") + if err := node.CopyFromLocal(brokenRpmOstree, "/tmp/rpm-ostree.broken"); err != nil { + return fmt.Errorf("error copying %s to node %s: %w", brokenRpmOstree, node, err) + } + _, err := node.DebugNodeWithChroot("sh", "-c", + "chmod +x /tmp/rpm-ostree.broken; "+ + "cp /usr/bin/rpm-ostree /tmp/rpm-ostree; "+ + "nsenter --mount=/proc/1/ns/mnt mount --bind /tmp/rpm-ostree.broken /usr/bin/rpm-ostree; "+ + "restorecon -v /usr/bin/rpm-ostree") + return err } // FixRebaseInNode fixes the rpm-ostree rebase process in a node func FixRebaseInNode(node *Node) error { - return RestoreRpmOstree(node) + logger.Infof("Fixing rpm-ostree rebase process in node %s", node.GetName()) + + _, err := node.DebugNodeWithChroot("sh", "-c", "pkill -9 -x rpm-ostree; nsenter --mount=/proc/1/ns/mnt umount /usr/bin/rpm-ostree") + return err } // GetOperatorNode returns the node running the MCO operator pod @@ -1603,57 +1615,3 @@ func FilterSchedulableNodesOrFail(nodes []*Node) []*Node { } return returnNodes } - -const ( - fakeRpmOstreeRemotePath = "/var/tmp/rpm-ostree-fake.sh" - fakeRpmRemotePath = "/var/tmp/rpm-fake.sh" -) - -// ReplaceRpmOstree copies a local script to the node and bind-mounts it over /usr/bin/rpm-ostree. -// The original binary is backed up to /var/tmp/rpm-ostree. -func ReplaceRpmOstree(node *Node, localScriptPath string) error { - logger.Infof("Replacing rpm-ostree with %s on node %s", localScriptPath, node.GetName()) - if err := node.CopyFromLocal(localScriptPath, fakeRpmOstreeRemotePath); err != nil { - return fmt.Errorf("error copying %s to node %s: %w", localScriptPath, node, err) - } - _, err := node.DebugNodeWithChroot("sh", "-c", - "chmod +x "+fakeRpmOstreeRemotePath+" && "+ - "cp /usr/bin/rpm-ostree /var/tmp/rpm-ostree && "+ - "nsenter --mount=/proc/1/ns/mnt mount --bind "+fakeRpmOstreeRemotePath+" /usr/bin/rpm-ostree && "+ - "restorecon -v /usr/bin/rpm-ostree") - return err -} - -// RestoreRpmOstree unmounts the bind-mounted rpm-ostree and removes backup and fake script files. -func RestoreRpmOstree(node *Node) error { - logger.Infof("Restoring rpm-ostree on node %s", node.GetName()) - _, err := node.DebugNodeWithChroot("sh", "-c", - "pkill -9 -x rpm-ostree; "+ - "nsenter --mount=/proc/1/ns/mnt umount -l /usr/bin/rpm-ostree 2>/dev/null; "+ - "rm -f /var/tmp/rpm-ostree "+fakeRpmOstreeRemotePath) - return err -} - -// ReplaceRpm copies a local script to the node and bind-mounts it over /usr/bin/rpm. -// The original binary is backed up to /var/tmp/rpm-real. -func ReplaceRpm(node *Node, localScriptPath string) error { - logger.Infof("Replacing rpm with %s on node %s", localScriptPath, node.GetName()) - if err := node.CopyFromLocal(localScriptPath, fakeRpmRemotePath); err != nil { - return fmt.Errorf("error copying %s to node %s: %w", localScriptPath, node, err) - } - _, err := node.DebugNodeWithChroot("sh", "-c", - "chmod +x "+fakeRpmRemotePath+" && "+ - "cp /usr/bin/rpm /var/tmp/rpm-real && "+ - "nsenter --mount=/proc/1/ns/mnt mount --bind "+fakeRpmRemotePath+" /usr/bin/rpm && "+ - "restorecon -v /usr/bin/rpm") - return err -} - -// RestoreRpm unmounts the bind-mounted rpm and removes backup and fake script files. -func RestoreRpm(node *Node) error { - logger.Infof("Restoring rpm on node %s", node.GetName()) - _, err := node.DebugNodeWithChroot("sh", "-c", - "nsenter --mount=/proc/1/ns/mnt umount -l /usr/bin/rpm 2>/dev/null; "+ - "rm -f /var/tmp/rpm-real "+fakeRpmRemotePath) - return err -} diff --git a/test/extended-priv/resource.go b/test/extended-priv/resource.go index ad0b35c68d..79c1030c3b 100644 --- a/test/extended-priv/resource.go +++ b/test/extended-priv/resource.go @@ -261,17 +261,9 @@ func (r *Resource) DeleteOrFail(extraParams ...string) { o.Expect(err).NotTo(o.HaveOccurred()) } -// GetSpecOrFail returns the resource's spec as a JSON string and fails the test if any error happens +// GetSpecOrFail returns the resource's spec as a JSON string func (r Resource) GetSpecOrFail() string { - spec, err := r.GetSpec() - o.Expect(err).NotTo(o.HaveOccurred()) - - return spec -} - -// GetSpec returns the resource's spec as a JSON string -func (r Resource) GetSpec() (string, error) { - return r.Get(`{.spec}`) + return r.GetOrFail(`{.spec}`) } // SetSpec replace the current resource's spec with the provided JSON string spec diff --git a/test/extended-priv/testdata/files/rpm-fake-usbguard-missing.sh b/test/extended-priv/testdata/files/rpm-fake-usbguard-missing.sh deleted file mode 100644 index 2e18c4ee0f..0000000000 --- a/test/extended-priv/testdata/files/rpm-fake-usbguard-missing.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/bin/bash - -# Fake rpm that reports usbguard as not installed. -# All other queries are forwarded to the real rpm backup. -if [ "$1" = "-q" ] && [ "$2" = "usbguard" ]; then - exit 1 -fi -/var/tmp/rpm-real "$@" diff --git a/test/extended-priv/testdata/files/rpm-ostree-fake-install-noop.sh b/test/extended-priv/testdata/files/rpm-ostree-fake-install-noop.sh deleted file mode 100644 index 9efab38a48..0000000000 --- a/test/extended-priv/testdata/files/rpm-ostree-fake-install-noop.sh +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/bash - -# Fake rpm-ostree that silently succeeds on install/override commands. -# All other commands are forwarded to the real rpm-ostree backup. -if [[ "$@" == *"install"* ]] || [[ "$@" == *"override"* ]]; then - echo "Installing package (fake)" - exit 0 -fi -exec /var/tmp/rpm-ostree "$@" diff --git a/test/extended-priv/util.go b/test/extended-priv/util.go index e065976995..7ed1ae7aae 100644 --- a/test/extended-priv/util.go +++ b/test/extended-priv/util.go @@ -320,18 +320,6 @@ func IsTrue(s string) bool { return strings.EqualFold(s, TrueString) } -// SafeCleanup executes the given cleanup function ensuring all statements run even if some assertions fail. -// It collects all gomega failures and reports them at the end, failing the test if any occurred. -// Use this in defer blocks instead of raw o.Expect calls to prevent cleanup from being interrupted. -func SafeCleanup(cleanup func()) { - errors := o.InterceptGomegaFailures(cleanup) - for _, e := range errors { - logger.Errorf("Cleanup error: %s", e) - } - o.Expect(len(errors)).To(o.BeZero(), - "There were %d errors during cleanup:\n%s", len(errors), strings.Join(errors, "\n")) -} - // IsSNOSafe returns true if the cluster is a SNO cluster. Instead of failing, it returns an error if we can't know if the cluster is SNO or not func IsSNOSafe(oc *exutil.CLI) (bool, error) { allNodes, err := NewNodeList(oc.AsAdmin()).GetAll() diff --git a/test/extended-priv/util/clusters.go b/test/extended-priv/util/clusters.go index 627e120d48..d93b4e8ff1 100644 --- a/test/extended-priv/util/clusters.go +++ b/test/extended-priv/util/clusters.go @@ -104,20 +104,3 @@ func SkipOnSingleNodeTopology(oc *CLI) { e2eskipper.Skipf("This test does not apply to single-node topologies") } } - -// SkipIfNoMAPIMachineSets skips the test if the cluster has no legacy MAPI MachineSets -// (machinesets.machine.openshift.io) in the openshift-machine-api namespace. Clusters that -// have migrated compute machine management to Cluster API no longer have any MAPI MachineSets, -// only CAPI ones under cluster.x-k8s.io in the openshift-cluster-api namespace, which these tests -// do not yet support. -func SkipIfNoMAPIMachineSets(oc *CLI) { - out, err := oc.AsAdmin().WithoutNamespace().Run("get").Args( - "machinesets.machine.openshift.io", "-n", "openshift-machine-api", - "-o", "jsonpath={range .items[*]}{.metadata.name}{end}", - ).Output() - o.Expect(err).NotTo(o.HaveOccurred(), "failed to list machinesets") - if out == "" { - e2eskipper.Skipf("No MAPI MachineSets (machinesets.machine.openshift.io) found in openshift-machine-api; " + - "this test does not yet support clusters where compute machine management has been migrated to Cluster API") - } -} diff --git a/test/extended-priv/util/pods.go b/test/extended-priv/util/pods.go index 61bd2ffb37..00ef4950eb 100644 --- a/test/extended-priv/util/pods.go +++ b/test/extended-priv/util/pods.go @@ -189,28 +189,3 @@ func GetAllPodsWithLabel(oc *CLI, namespace, label string) ([]string, error) { } return strings.Split(pods, " "), err } - -// AssertAllNonJobPodsToBeReadyWithPollerParams asserts all non-job pods in a namespace are ready within the specified timeout -// Exclude transient Job pods ('job-name') from readiness: build Job pods are short-lived and not expected to be Ready=True, so including them causes false failure -// we validate the Job via logs/conditions separately, and this gate only asserts persistent controller pods (e.g., machine-os-builder) are Ready. -func AssertAllNonJobPodsToBeReadyWithPollerParams(oc *CLI, namespace string, interval, timeout time.Duration) { - ctx := context.Background() - err := wait.PollUntilContextTimeout(ctx, interval, timeout, true, func(_ context.Context) (bool, error) { - - // get the status flag for all pods - // except the ones which are in Complete Status. - // exclude pods that belong to Jobs (pods with 'job-name' label) - // it use 'ne' operator which is only compatible with 4.10+ oc versions - template := "'{{- range .items -}}{{- range .status.conditions -}}{{- if ne .reason \"PodCompleted\" -}}{{- if eq .type \"Ready\" -}}{{- .status}} {{\" \"}}{{- end -}}{{- end -}}{{- end -}}{{- end -}}'" - stdout, err := oc.AsAdmin().Run("get").Args("pods", "-n", namespace, "-l", "!job-name").Template(template).Output() - if err != nil { - e2e.Logf("the err:%v, and try next round", err) - return false, nil - } - if strings.Contains(stdout, "False") { - return false, nil - } - return true, nil - }) - AssertWaitPollNoErr(err, fmt.Sprintf("Some Pods are not ready in NS %s (excluding Job pods)!", namespace)) -} diff --git a/test/extended/osImageStream.go b/test/extended/osImageStream.go index d624fc14aa..7a22532189 100644 --- a/test/extended/osImageStream.go +++ b/test/extended/osImageStream.go @@ -13,6 +13,7 @@ import ( logger "github.com/openshift/machine-config-operator/test/extended-priv/util/logext" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" ) const ( @@ -32,8 +33,8 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/disruptive ) g.JustBeforeEach(func() { - // Skip if no MAPI MachineSets are available (e.g. compute machine management migrated to CAPI) - exutil.SkipIfNoMAPIMachineSets(oc.AsAdmin()) + // Skip this test if the cluster is not using MachineAPI + skipUnlessFunctionalMachineAPI(oc) }) g.It("Machines, MachineSets, and ControlPlaneMachineSets (if applicable) are labeled with OSStream [apigroup:machineconfiguration.openshift.io]", func() { @@ -120,6 +121,34 @@ func validateOSStreamLabelOnControlPlaneMachineSet(machineClient *machineclient. return hasLabel && slices.Contains(expectedOSStreams, osStream) && hasLabelTemp && slices.Contains(expectedOSStreams, osStreamTemp) } +// skipUnlessFunctionalMachineAPI skips the test if the cluster is not using Machine API +func skipUnlessFunctionalMachineAPI(oc *exutil.CLI) { + machineClient, err := machineclient.NewForConfig(oc.KubeFramework().ClientConfig()) + o.Expect(err).ToNot(o.HaveOccurred()) + machines, err := machineClient.MachineV1beta1().Machines(MAPINamespace).List(context.Background(), metav1.ListOptions{LabelSelector: MAPIMasterMachineLabelSelector}) + // the machine API can be unavailable resulting in a 404 or an empty list + if err != nil { + if !apierrors.IsNotFound(err) { + o.Expect(err).ToNot(o.HaveOccurred()) + } + g.Skip("haven't found machines resources on the cluster, this test can be run on a platform that supports functional MachineAPI") + return + } + if len(machines.Items) == 0 { + g.Skip("got an empty list of machines resources from the cluster, this test can be run on a platform that supports functional MachineAPI") + return + } + + // we expect just a single machine to be in the Running state + for _, machine := range machines.Items { + phase := ptr.Deref(machine.Status.Phase, "") + if phase == "Running" { + return + } + } + g.Skip("haven't found a machine in running state, this test can be run on a platform that supports functional MachineAPI") +} + // getAllMachineSets returns all the MachineSets in a cluster func getAllMachineSets(machineClient *machineclient.Clientset) *machinev1beta1.MachineSetList { machineSets, err := machineClient.MachineV1beta1().MachineSets("openshift-machine-api").List(context.TODO(), metav1.ListOptions{})