added tests

Author: dkhater-redhat

pkg/controller/build/osbuildcontroller_test.go

@@ -756,3 +756,36 @@ func assertMachineOSConfigGetsCurrentBuildAnnotation(ctx context.Context, t *tes
 
 	require.NoError(t, err)
 }
+
+// Test that when the MCP’s rendered-MC name changes but the two MCs only differ
+// by on-cluster layering, no new MachineOSBuild or Build Job is created.
+func TestOSBuildControllerSkipsBuildForLayerOnlyChanges(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	t.Cleanup(cancel)
+	poolName := "worker"
+
+	_, mcfgclient, _, _, mosc, firstMosb, mcp, kubeassert := setupOSBuildControllerForTestWithSuccessfulBuild(ctx, t, poolName)
+	isMachineOSBuildReachedExpectedCount(ctx, t, mcfgclient, mosc, 1)
+
+	assertMachineOSConfigGetsCurrentBuildAnnotation(ctx, t, mcfgclient, mosc, firstMosb)
+
+	isMachineOSBuildReachedExpectedCount(ctx, t, mcfgclient, mosc, 1)
+
+	insertNewRenderedMachineConfigAndUpdatePool(ctx, t, mcfgclient, mcp.Name, "rendered-worker-layer-only")
+
+	// Give the controller a moment to (not) kick off a build
+	time.Sleep(200 * time.Millisecond)
+
+	mosbList, err := mcfgclient.MachineconfigurationV1().
+		MachineOSBuilds().
+		List(ctx, metav1.ListOptions{LabelSelector: utils.MachineOSBuildForPoolSelector(mosc).String()})
+	require.NoError(t, err)
+	require.Len(t, mosbList.Items, 2, "expected a new MOSB to be created for layering-only change")
+	assert.Equal(t, firstMosb.Name, mosbList.Items[0].Name, "first MOSB should remain unchanged")
+
+	layerOnlyMosb := mosbList.Items[1]
+
+	jobName := utils.GetBuildJobName(&layerOnlyMosb)
+
+	kubeassert.JobDoesNotExist(jobName, "layering-only MOSB should not spawn a build Job")
+}

pkg/controller/build/reconciler.go

@@ -1195,20 +1195,27 @@ func (b *buildReconciler) syncMachineOSBuild(ctx context.Context, mosb *mcfgv1.M
 		oldMCName := mcp.Status.Configuration.Name
 		newMCName := mcp.Spec.Configuration.Name
 
-		oldMC, err := b.machineConfigLister.Get(oldMCName)
+		oldRendered, err := b.machineConfigLister.Get(oldMCName)
 		if err != nil {
 			return err
 		}
-		newMC, err := b.machineConfigLister.Get(newMCName)
+		newRendered, err := b.machineConfigLister.Get(newMCName)
 		if err != nil {
 			return err
 		}
 
-		if !ctrlcommon.RequiresRebuild(oldMC, newMC) && oldMCName != newMCName {
+		old := mcp.DeepCopy()
+		old.Spec.Configuration.Name = mcp.Status.Configuration.Name
+
+		// decide if we annotate or reuse
+		needsImageRebuild, err := b.reconcileImageRebuild(old, mcp)
+		if err != nil {
+			return err
+		}
+		// need image rebuild = true -> isRebuild = false
+		if oldRendered != newRendered && !needsImageRebuild {
 			klog.Infof("[syncMachineOSBuild] MachineOSBuild %q: only layering changes → skipping builder", mosb.Name)
 			return nil
-		} else {
-			klog.Infof("[syncMachineOSBuild] MachineOSBuild %q: not skipping builder", mosb.Name)
 		}
 
 		mosbState := ctrlcommon.NewMachineOSBuildState(mosb)

pkg/daemon/daemon.go

@@ -2589,7 +2589,7 @@ func (dn *Daemon) triggerUpdate(currentConfig, desiredConfig *mcfgv1.MachineConf
 }
 
 // We might be able to drop this method entirely since I'm not sure if it's actually needed.
-func (dn *Daemon) triggerUpdateWithMachineConfig(currentConfig, desiredConfig *mcfgv1.MachineConfig, skipCertificateWrite bool) error {
+func (dn *Daemon) triggerUpdateWithMachineConfig(currentConfig, desiredConfig *mcfgv1.MachineConfig, _ bool) error {
 	return dn.triggerUpdate(currentConfig, desiredConfig, "", "")
 }