From 9e106a1854754c715cfd755da840bb6854b917ab Mon Sep 17 00:00:00 2001 From: Zack Zlotnik Date: Tue, 12 Nov 2024 15:59:51 -0500 Subject: [PATCH] MCO-1165: implements rebuild functionality By adding an annotation to a MachineOSConfig object, a cluster admin will be able to cause a rebuild to occur. --- pkg/controller/build/constants/constants.go | 6 ++ pkg/controller/build/helpers.go | 7 ++- .../build/osbuildcontroller_test.go | 28 +++++++++ pkg/controller/build/reconciler.go | 61 +++++++++++++++++-- .../e2e-techpreview/onclusterlayering_test.go | 24 +++++++- 5 files changed, 117 insertions(+), 9 deletions(-) diff --git a/pkg/controller/build/constants/constants.go b/pkg/controller/build/constants/constants.go index e75c0d240f..a24a3a9169 100644 --- a/pkg/controller/build/constants/constants.go +++ b/pkg/controller/build/constants/constants.go @@ -28,6 +28,12 @@ const ( CurrentMachineOSBuildAnnotationKey string = "machineconfiguration.openshift.io/current-machine-os-build" ) +// When this annotation is added to a MachineOSConfig, the current +// MachineOSBuild will be deleted, which will cause a rebuild to occur. +const ( + RebuildMachineOSConfigAnnotationKey string = "machineconfiguration.openshift.io/rebuild" +) + // Entitled build secret names const ( // Name of the etc-pki-entitlement secret from the openshift-config-managed namespace. diff --git a/pkg/controller/build/helpers.go b/pkg/controller/build/helpers.go index 72e5bfc53a..efba51007a 100644 --- a/pkg/controller/build/helpers.go +++ b/pkg/controller/build/helpers.go @@ -235,7 +235,7 @@ func isMachineOSBuildCurrentForMachineOSConfigWithPullspec(mosc *mcfgv1alpha1.Ma // Determines if a given MachineOSConfig has the current build annotation. func hasCurrentBuildAnnotation(mosc *mcfgv1alpha1.MachineOSConfig) bool { - return metav1.HasAnnotation(mosc.ObjectMeta, constants.CurrentMachineOSBuildAnnotationKey) + return metav1.HasAnnotation(mosc.ObjectMeta, constants.CurrentMachineOSBuildAnnotationKey) && mosc.Annotations[constants.CurrentMachineOSBuildAnnotationKey] != "" } // Determines if a given MachineOSConfig has the current build annotation and @@ -248,6 +248,11 @@ func isCurrentBuildAnnotationEqual(mosc *mcfgv1alpha1.MachineOSConfig, mosb *mcf return mosc.Annotations[constants.CurrentMachineOSBuildAnnotationKey] == mosb.Name } +// Determines if a given MachineOSConfig has the rebuild annotation. +func hasRebuildAnnotation(mosc *mcfgv1alpha1.MachineOSConfig) bool { + return metav1.HasAnnotation(mosc.ObjectMeta, constants.RebuildMachineOSConfigAnnotationKey) +} + // Looks at the error chain for the given error and determines if the error // should be ignored or not based upon whether it is a not found error. If it // should be ignored, this will log the error as well as the name and kind of diff --git a/pkg/controller/build/osbuildcontroller_test.go b/pkg/controller/build/osbuildcontroller_test.go index 8bdf408b8b..f86790b46a 100644 --- a/pkg/controller/build/osbuildcontroller_test.go +++ b/pkg/controller/build/osbuildcontroller_test.go @@ -434,6 +434,34 @@ func TestOSBuildController(t *testing.T) { }) } +// Validates that when a MachineOSConfig gets the rebuild annotation that the +// MachineOSBuild associated with it is deleted and then rebuilt. +func TestOSBuildControllerRebuildAnnotation(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) + t.Cleanup(cancel) + + _, mcfgclient, mosc, mosb, _, kubeassert := setupOSBuildControllerForTestWithSuccessfulBuild(ctx, t, "worker") + assertBuildObjectsAreDeleted(ctx, t, kubeassert, mosb) + + apiMosc, err := mcfgclient.MachineconfigurationV1alpha1().MachineOSConfigs().Get(ctx, mosc.Name, metav1.GetOptions{}) + require.NoError(t, err) + + apiMosc.Annotations[constants.RebuildMachineOSConfigAnnotationKey] = "" + + _, err = mcfgclient.MachineconfigurationV1alpha1().MachineOSConfigs().Update(ctx, apiMosc, metav1.UpdateOptions{}) + require.NoError(t, err) + + assertBuildObjectsAreCreated(ctx, t, kubeassert, mosb) + + apiMosc, err = mcfgclient.MachineconfigurationV1alpha1().MachineOSConfigs().Get(ctx, mosc.Name, metav1.GetOptions{}) + require.NoError(t, err) + + assert.NotContains(t, apiMosc.GetAnnotations(), constants.RebuildMachineOSConfigAnnotationKey) + +} + func assertBuildObjectsAreCreated(ctx context.Context, t *testing.T, kubeassert *testhelpers.Assertions, mosb *mcfgv1alpha1.MachineOSBuild) { t.Helper() diff --git a/pkg/controller/build/reconciler.go b/pkg/controller/build/reconciler.go index 13713e2ccf..9feb1154d7 100644 --- a/pkg/controller/build/reconciler.go +++ b/pkg/controller/build/reconciler.go @@ -91,6 +91,15 @@ func (b *buildReconciler) UpdateMachineOSConfig(ctx context.Context, old, cur *m // Executes whenever a MachineOSConfig is updated. If the build inputs have // changed, a new MachineOSBuild should be created. func (b *buildReconciler) updateMachineOSConfig(ctx context.Context, old, cur *mcfgv1alpha1.MachineOSConfig) error { + // If we have gained the rebuild annotation, we should delete the current MachineOSBuild associated with this MachineOSConfig. + if !hasRebuildAnnotation(old) && hasRebuildAnnotation(cur) { + if err := b.rebuildMachineOSConfig(ctx, cur); err != nil { + return fmt.Errorf("could not rebuild MachineOSConfig %q: %w", cur.Name, err) + } + + return nil + } + // Whenever the build inputs have changed, create a new MachineOSBuild. if !equality.Semantic.DeepEqual(old.Spec.BuildInputs, cur.Spec.BuildInputs) { klog.Infof("Detected MachineOSConfig change for %s", cur.Name) @@ -100,6 +109,37 @@ func (b *buildReconciler) updateMachineOSConfig(ctx context.Context, old, cur *m return b.syncMachineOSConfigs(ctx) } +// Rebuilds the most current build associated with a MachineOSConfig whenever +// the rebuild annotation is applied. This is done by deleting the current +// MachineOSBuild and allowing the controller to replace it with a new one. +func (b *buildReconciler) rebuildMachineOSConfig(ctx context.Context, mosc *mcfgv1alpha1.MachineOSConfig) error { + klog.Infof("MachineOSConfig %q has rebuild annotation (%q)", mosc.Name, constants.RebuildMachineOSConfigAnnotationKey) + + if !hasCurrentBuildAnnotation(mosc) { + klog.Infof("MachineOSConfig %q does not have current build annotation (%q) set, skipping rebuild", mosc.Name, constants.CurrentMachineOSBuildAnnotationKey) + return nil + } + + mosbName := mosc.Annotations[constants.CurrentMachineOSBuildAnnotationKey] + + mosb, err := b.machineOSBuildLister.Get(mosbName) + if err != nil { + return ignoreErrIsNotFound(fmt.Errorf("cannot rebuild MachineOSConfig %q: %w", mosc.Name, err)) + } + + if err := b.deleteMachineOSBuild(ctx, mosb); err != nil { + return fmt.Errorf("could not delete MachineOSBuild %q for MachineOSConfig %q: %w", mosb.Name, mosc.Name, err) + } + + if err := b.createNewMachineOSBuildOrReuseExisting(ctx, mosc); err != nil { + return fmt.Errorf("could not create new MachineOSBuild for MachineOSConfig %q: %w", mosc.Name, err) + } + + klog.Infof("MachineOSConfig %q is now rebuilding", mosc.Name) + + return nil +} + // Runs whenever a new MachineOSConfig is added. Determines if a new // MachineOSBuild should be created and then creates it, if needed. func (b *buildReconciler) addMachineOSConfig(ctx context.Context, mosc *mcfgv1alpha1.MachineOSConfig) error { @@ -233,23 +273,34 @@ func (b *buildReconciler) updateMachineOSConfigStatus(ctx context.Context, mosc return err } + annoUpdateNeeded := false + + if hasRebuildAnnotation(mosc) { + delete(mosc.Annotations, constants.RebuildMachineOSConfigAnnotationKey) + annoUpdateNeeded = true + klog.Infof("Cleared rebuild annotation (%q) on MachineOSConfig %q", constants.RebuildMachineOSConfigAnnotationKey, mosc.Name) + } + if !isCurrentBuildAnnotationEqual(mosc, mosb) { - // If the current build annotation is not equal, do an update and reuse the - // returned MachineOSBuild for the status update, if needed. metav1.SetMetaDataAnnotation(&mosc.ObjectMeta, constants.CurrentMachineOSBuildAnnotationKey, mosb.Name) + annoUpdateNeeded = true + klog.Infof("Set current build on MachineOSConfig %q to MachineOSBuild %q", mosc.Name, mosb.Name) + } + + if annoUpdateNeeded { updatedMosc, err := b.mcfgclient.MachineconfigurationV1alpha1().MachineOSConfigs().Update(ctx, mosc, metav1.UpdateOptions{}) if err != nil { - return fmt.Errorf("could not set current build annotation on MachineOSConfig %q: %w", mosc.Name, err) + return fmt.Errorf("could not update annotations on MachineOSConfig %q: %w", mosc.Name, err) } - klog.Infof("Set current build on MachineOSConfig %q to MachineOSBuild %q", mosc.Name, mosb.Name) + klog.Infof("Updated annotations on MachineOSConfig %q", mosc.Name) mosc = updatedMosc } // Skip the status update if final image pushspec hasn't been set yet. if mosb.Status.FinalImagePushspec == "" { - klog.Infof("MachineOSBuild %q has empty final image pushspec, skipping MachineOSConfig %q update", mosb.Name, mosc.Name) + klog.Infof("MachineOSBuild %q has empty final image pushspec, skipping MachineOSConfig %q status update", mosb.Name, mosc.Name) return nil } diff --git a/test/e2e-techpreview/onclusterlayering_test.go b/test/e2e-techpreview/onclusterlayering_test.go index f8a781b9e9..74a7f1bed4 100644 --- a/test/e2e-techpreview/onclusterlayering_test.go +++ b/test/e2e-techpreview/onclusterlayering_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/require" "github.com/openshift/machine-config-operator/pkg/controller/build/buildrequest" + "github.com/openshift/machine-config-operator/pkg/controller/build/constants" "github.com/openshift/machine-config-operator/pkg/controller/build/utils" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -90,7 +91,7 @@ type onClusterLayeringTestOpts struct { useYumRepos bool } -func TestOnClusterBuildsOnOKD(t *testing.T) { +func TestOnClusterLayeringOnOKD(t *testing.T) { skipOnOCP(t) runOnClusterLayeringTest(t, onClusterLayeringTestOpts{ @@ -102,18 +103,35 @@ func TestOnClusterBuildsOnOKD(t *testing.T) { } // Tests that an on-cluster build can be performed with the Custom Pod Builder. -func TestOnClusterBuildsCustomPodBuilder(t *testing.T) { +func TestOnClusterLayering(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + runOnClusterLayeringTest(t, onClusterLayeringTestOpts{ poolName: layeredMCPName, customDockerfiles: map[string]string{ layeredMCPName: cowsayDockerfile, }, }) + + t.Logf("Applying rebuild annotation (%q) to MachineOSConfig (%q) to cause a rebuild", constants.RebuildMachineOSConfigAnnotationKey, layeredMCPName) + + cs := framework.NewClientSet("") + + mosc, err := cs.MachineconfigurationV1alpha1Interface.MachineOSConfigs().Get(ctx, layeredMCPName, metav1.GetOptions{}) + require.NoError(t, err) + + mosc.Annotations[constants.RebuildMachineOSConfigAnnotationKey] = "" + + _, err = cs.MachineconfigurationV1alpha1Interface.MachineOSConfigs().Update(ctx, mosc, metav1.UpdateOptions{}) + require.NoError(t, err) + + waitForBuildToStartForPoolAndConfig(t, cs, layeredMCPName, layeredMCPName) } // Tests that an on-cluster build can be performed and that the resulting image // is rolled out to an opted-in node. -func TestOnClusterBuildRollsOutImage(t *testing.T) { +func TestOnClusterLayeringRollsOutImage(t *testing.T) { imagePullspec := runOnClusterLayeringTest(t, onClusterLayeringTestOpts{ poolName: layeredMCPName, customDockerfiles: map[string]string{