Skip to content

Commit b3d97fc

Browse files
dkhater-redhatdkhater-redhat
committed
reboot work
1 parent b7b1294 commit b3d97fc

File tree

10 files changed

+280
-88
lines changed

10 files changed

+280
-88
lines changed

‎pkg/controller/build/clients.go

+6
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
type informers struct {
2424
controllerConfigInformer mcfginformersv1.ControllerConfigInformer
2525
machineConfigPoolInformer mcfginformersv1.MachineConfigPoolInformer
26+
machineConfigInformer mcfginformersv1.MachineConfigInformer
2627
jobInformer batchinformersv1.JobInformer
2728
machineOSBuildInformer mcfginformersv1.MachineOSBuildInformer
2829
machineOSConfigInformer mcfginformersv1.MachineOSConfigInformer
@@ -43,6 +44,7 @@ func (i *informers) listers() *listers {
4344
machineOSBuildLister: i.machineOSBuildInformer.Lister(),
4445
machineOSConfigLister: i.machineOSConfigInformer.Lister(),
4546
machineConfigPoolLister: i.machineConfigPoolInformer.Lister(),
47+
machineConfigLister: i.machineConfigInformer.Lister(),
4648
jobLister: i.jobInformer.Lister(),
4749
controllerConfigLister: i.controllerConfigInformer.Lister(),
4850
}
@@ -53,6 +55,7 @@ type listers struct {
5355
machineOSBuildLister mcfglistersv1.MachineOSBuildLister
5456
machineOSConfigLister mcfglistersv1.MachineOSConfigLister
5557
machineConfigPoolLister mcfglistersv1.MachineConfigPoolLister
58+
machineConfigLister mcfglistersv1.MachineConfigLister
5659
jobLister batchlisterv1.JobLister
5760
controllerConfigLister mcfglistersv1.ControllerConfigLister
5861
}
@@ -84,6 +87,7 @@ func newInformers(mcfgclient mcfgclientset.Interface, kubeclient clientset.Inter
8487
controllerConfigInformer := mcoInformerFactory.Machineconfiguration().V1().ControllerConfigs()
8588
machineConfigPoolInformer := mcoInformerFactory.Machineconfiguration().V1().MachineConfigPools()
8689
machineOSBuildInformer := mcoInformerFactory.Machineconfiguration().V1().MachineOSBuilds()
90+
machineConfigInformer := mcoInformerFactory.Machineconfiguration().V1().MachineConfigs()
8791
machineOSConfigInformer := mcoInformerFactory.Machineconfiguration().V1().MachineOSConfigs()
8892
jobInformer := coreInformerFactory.Batch().V1().Jobs()
8993

@@ -92,6 +96,7 @@ func newInformers(mcfgclient mcfgclientset.Interface, kubeclient clientset.Inter
9296
machineConfigPoolInformer: machineConfigPoolInformer,
9397
machineOSBuildInformer: machineOSBuildInformer,
9498
machineOSConfigInformer: machineOSConfigInformer,
99+
machineConfigInformer: machineConfigInformer,
95100
jobInformer: jobInformer,
96101
toStart: []interface{ Start(<-chan struct{}) }{
97102
mcoInformerFactory,
@@ -103,6 +108,7 @@ func newInformers(mcfgclient mcfgclientset.Interface, kubeclient clientset.Inter
103108
jobInformer.Informer().HasSynced,
104109
machineOSBuildInformer.Informer().HasSynced,
105110
machineOSConfigInformer.Informer().HasSynced,
111+
machineConfigInformer.Informer().HasSynced,
106112
},
107113
}
108114
}

‎pkg/controller/build/constants/constants.go

+5
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ const (
2929
CurrentMachineOSBuildAnnotationKey string = "machineconfiguration.openshift.io/current-machine-os-build"
3030
)
3131

32+
// RenderedMCWithImageLabel is the label that signifies an MC change requires an OCL image change and node reboot
33+
const (
34+
RenderedMCWithImageLabel = "machineconfiguration.openshift.io/rendered-mc-with-image"
35+
)
36+
3237
// When this annotation is added to a MachineOSConfig, the current
3338
// MachineOSBuild will be deleted, which will cause a rebuild to occur.
3439
const (

‎pkg/controller/build/reconciler.go

+151-17
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func (b *buildReconciler) updateMachineOSConfig(ctx context.Context, old, cur *m
102102

103103
// Whenever the MachineOSConfig spec has changed, create a new MachineOSBuild.
104104
if !equality.Semantic.DeepEqual(old.Spec, cur.Spec) {
105-
klog.Infof("Detected MachineOSConfig change for %s", cur.Name)
105+
klog.Infof("[updateMachineOSConfig] Detected MachineOSConfig change and kicking off createNewMachineOSBuildOrReuseExisting for %s", cur.Name)
106106
return b.createNewMachineOSBuildOrReuseExisting(ctx, cur, false)
107107
}
108108

@@ -316,7 +316,7 @@ func (b *buildReconciler) updateMachineOSConfigStatus(ctx context.Context, mosc
316316

317317
// skip the status update if the current image pullspec equals the digest image pushspec.
318318
if mosc.Status.CurrentImagePullSpec == mosb.Status.DigestedImagePushSpec {
319-
klog.Infof("MachineOSConfig %q already has final image pushspec for MachineOSBuild %q", mosc.Name, mosb.Name)
319+
klog.Infof("[updateMachineOSConfig] MachineOSConfig %q already has final image pushspec for MachineOSBuild %q", mosc.Name, mosb.Name)
320320
return nil
321321
}
322322

@@ -356,16 +356,41 @@ func (b *buildReconciler) UpdateMachineConfigPool(ctx context.Context, oldMCP, c
356356
// Sepcifically, whenever a new rendered MachineConfig is applied, it will
357357
// create a new MachineOSBuild in response.
358358
func (b *buildReconciler) updateMachineConfigPool(ctx context.Context, oldMCP, curMCP *mcfgv1.MachineConfigPool) error {
359-
if oldMCP.Spec.Configuration.Name != curMCP.Spec.Configuration.Name {
360-
klog.Infof("Rendered config for pool %s changed from %s to %s", curMCP.Name, oldMCP.Spec.Configuration.Name, curMCP.Spec.Configuration.Name)
361-
if err := b.createNewMachineOSBuildOrReuseExistingForPoolChange(ctx, curMCP); err != nil {
362-
return fmt.Errorf("could not create or reuse existing MachineOSBuild for MachineConfigPool %q change: %w", curMCP.Name, err)
363-
}
359+
360+
// Did the rendered-MC actually change?
361+
_, reuse, err := b.reconcilePoolChange(oldMCP, curMCP)
362+
if err != nil {
363+
return err
364+
}
365+
366+
/////////// REMOVE JUST FOR LOGS ///////////
367+
// Get current and desired MachineConfigs
368+
curMC, err := b.machineConfigLister.Get(oldMCP.Spec.Configuration.Name)
369+
if err != nil {
370+
return fmt.Errorf("error getting current MachineConfig %s: %w", oldMCP.Spec.Configuration.Name, err)
371+
}
372+
373+
desMC, err := b.machineConfigLister.Get(curMCP.Spec.Configuration.Name)
374+
if err != nil {
375+
return fmt.Errorf("error getting desired MachineConfig %s: %w", curMCP.Spec.Configuration.Name, err)
364376
}
365377

366-
// Not sure if we need to do this here yet or not.
367-
// TODO: Determine if we should call b.syncMachineConfigPools() here or not.
368-
return b.syncAll(ctx)
378+
klog.Infof("desMC is %s", desMC.Name)
379+
klog.Infof("currMC is %s", curMC.Name)
380+
/////////// REMOVE JUST FOR LOGS ///////////
381+
382+
if reuse {
383+
return b.handleReuseMachineOSBuild(ctx, curMCP)
384+
}
385+
return b.createNewMachineOSBuildOrReuseExistingForPoolChange(ctx, curMCP)
386+
387+
}
388+
389+
func (b *buildReconciler) patchMOSBLabels(ctx context.Context, mosb *mcfgv1.MachineOSBuild, newMCName string) error {
390+
mosbCopy := mosb.DeepCopy()
391+
mosbCopy.Labels[constants.RenderedMachineConfigLabelKey] = newMCName
392+
_, err := b.mcfgclient.MachineconfigurationV1().MachineOSBuilds().Update(ctx, mosbCopy, metav1.UpdateOptions{})
393+
return err
369394
}
370395

371396
// Adds a MachineOSBuild.
@@ -927,9 +952,54 @@ func (b *buildReconciler) syncMachineOSBuild(ctx context.Context, mosb *mcfgv1.M
927952
return fmt.Errorf("could not get MachineConfigPool from MachineOSBuild %q: %w", mosb.Name, err)
928953
}
929954

930-
// An mosb which had previously been forgotten by the queue and is no longer desired by the mcp should not build
931-
if mosb.ObjectMeta.Labels[constants.RenderedMachineConfigLabelKey] != mcp.Spec.Configuration.Name {
932-
klog.Infof("The MachineOSBuild %q which builds the rendered Machine Config %q is no longer desired by the MCP %q", mosb.Name, mosb.ObjectMeta.Labels[constants.RenderedMachineConfigLabelKey], mosb.ObjectMeta.Labels[constants.TargetMachineConfigPoolLabelKey])
955+
desiredMC, err := b.machineConfigLister.Get(mcp.Spec.Configuration.Name)
956+
if err != nil {
957+
return fmt.Errorf("error getting desired MC: %w", err)
958+
}
959+
960+
// existing MC labels
961+
currentWithImage := mosb.Labels[constants.RenderedMCWithImageLabel]
962+
currMC := mosb.Labels[constants.RenderedMachineConfigLabelKey]
963+
964+
oldMC, err := b.machineConfigLister.Get(currMC)
965+
if err != nil {
966+
return fmt.Errorf("could not fetch old MachineConfig %q: %w", currMC, err)
967+
}
968+
969+
// new MC labels
970+
desMC := desiredMC.Name
971+
newWithImage := currentWithImage
972+
973+
mosbCopy := mosb.DeepCopy()
974+
updateLabels := false
975+
976+
if currMC != desMC {
977+
klog.Info("Current MC does not equal the new MC, we need to update our labels to reflect that")
978+
mosbCopy.Labels[constants.RenderedMachineConfigLabelKey] = desMC
979+
updateLabels = true
980+
}
981+
982+
if ctrlcommon.RequiresRebuild(oldMC, desiredMC) && currentWithImage != newWithImage {
983+
mosbCopy.Labels[constants.RenderedMCWithImageLabel] = newWithImage
984+
updateLabels = true
985+
}
986+
987+
if updateLabels {
988+
klog.Infof("syncMachineOSBuild: Updating labels for MOSB %q (currentNoImage=%s, desiredMCName=%s, desiredWithImage=%s, currentWithImage=%s)",
989+
mosb.Name, currMC, desMC, newWithImage, currentWithImage)
990+
991+
mosbCopy.Labels[constants.RenderedMachineConfigLabelKey] = desMC
992+
mosbCopy.Labels[constants.RenderedMCWithImageLabel] = newWithImage
993+
994+
// Update the MOSB object
995+
_, err := b.mcfgclient.MachineconfigurationV1().MachineOSBuilds().Update(ctx, mosbCopy, metav1.UpdateOptions{})
996+
if err != nil {
997+
klog.Errorf("syncMachineOSBuild: Failed to update MOSB %q labels: %v", mosb.Name, err)
998+
999+
return fmt.Errorf("failed to sync labels: %v", err)
1000+
}
1001+
klog.Infof("syncMachineOSBuild: Updated MOSB %q labels to RenderedMCNoImageLabel=%s, RenderedMCWithImageLabel=%s",
1002+
mosb.Name, desiredMC.Name, newWithImage)
9331003
return nil
9341004
}
9351005

@@ -1029,7 +1099,7 @@ func (b *buildReconciler) syncMachineOSConfig(ctx context.Context, mosc *mcfgv1.
10291099
}
10301100
}
10311101

1032-
klog.Infof("No matching MachineOSBuild found for MachineOSConfig %q, will create one", mosc.Name)
1102+
klog.Infof("syncMachineOSConfig: No matching MachineOSBuild found for MachineOSConfig %q, will create one by kicking off createNewMachineOSBuildOrReuseExisting", mosc.Name)
10331103
if err := b.createNewMachineOSBuildOrReuseExisting(ctx, mosc, false); err != nil {
10341104
return fmt.Errorf("could not create new or reuse existing MachineOSBuild for MachineOSConfig %q: %w", mosc.Name, err)
10351105
}
@@ -1066,7 +1136,71 @@ func (b *buildReconciler) syncMachineConfigPools(ctx context.Context) error {
10661136
// MachineOSConfigs and MachineOSBuilds, which will create a new MachineOSBuild,
10671137
// if needed.
10681138
func (b *buildReconciler) syncMachineConfigPool(ctx context.Context, mcp *mcfgv1.MachineConfigPool) error {
1069-
return b.timeObjectOperation(mcp, syncingVerb, func() error {
1070-
return b.createNewMachineOSBuildOrReuseExistingForPoolChange(ctx, mcp)
1071-
})
1139+
_, err := utils.GetMachineOSConfigForMachineConfigPool(mcp, b.utilListers())
1140+
if err != nil {
1141+
// Not opted in yet, so nothing for us to do.
1142+
klog.V(4).Infof("buildReconciler: no MachineOSConfig for pool %q, skipping", mcp.Name)
1143+
return nil
1144+
}
1145+
// old pool
1146+
old := mcp.DeepCopy()
1147+
old.Spec.Configuration.Name = mcp.Status.Configuration.Name
1148+
1149+
// decide if we annotate or reuse
1150+
_, reuse, err := b.reconcilePoolChange(old, mcp)
1151+
if err != nil {
1152+
return err
1153+
}
1154+
1155+
if reuse {
1156+
return b.handleReuseMachineOSBuild(ctx, mcp)
1157+
1158+
}
1159+
return b.createNewMachineOSBuildOrReuseExistingForPoolChange(ctx, mcp)
1160+
1161+
}
1162+
1163+
func (b *buildReconciler) handleReuseMachineOSBuild(ctx context.Context, mcp *mcfgv1.MachineConfigPool) error {
1164+
klog.Infof("[handleReuseMachineOSBuild] Reusing last MOSB")
1165+
1166+
mosc, err := utils.GetMachineOSConfigForMachineConfigPool(mcp, b.utilListers())
1167+
if err != nil {
1168+
return fmt.Errorf("could not find MachineOSConfig for pool %q: %w", mcp.Name, err)
1169+
}
1170+
1171+
existingMosb, err := utils.GetMachineOSBuildForMachineConfigPool(mcp, b.utilListers())
1172+
if err != nil {
1173+
return fmt.Errorf("could not get MOSB for MCP %q: %w", mcp.Name, err)
1174+
}
1175+
klog.Infof("[handleReuseMachineOSBuild]: MC-only change, reusing MOSB %q for pool %q", existingMosb.Name, mcp.Name)
1176+
1177+
b.patchMOSBLabels(ctx, existingMosb, mcp.Spec.Configuration.Name)
1178+
1179+
return b.reuseExistingMachineOSBuildIfPossible(ctx, mosc, existingMosb)
1180+
1181+
}
1182+
1183+
// reconcilePoolChange returns (annotateRebuild bool, reuseOnly bool, err error)
1184+
func (b *buildReconciler) reconcilePoolChange(oldMCP, curMCP *mcfgv1.MachineConfigPool) (bool, bool, error) {
1185+
1186+
// if the old mc is the same as the new mc, no updated needed
1187+
if oldMCP.Spec.Configuration.Name == curMCP.Spec.Configuration.Name {
1188+
return false, false, nil
1189+
}
1190+
1191+
curr, err := b.machineConfigLister.Get(oldMCP.Spec.Configuration.Name)
1192+
if err != nil {
1193+
return false, false, err
1194+
}
1195+
des, err := b.machineConfigLister.Get(curMCP.Spec.Configuration.Name)
1196+
if err != nil {
1197+
return false, false, err
1198+
}
1199+
1200+
needsImageRebuild := ctrlcommon.RequiresRebuild(curr, des)
1201+
if needsImageRebuild {
1202+
return true, false, nil
1203+
}
1204+
1205+
return false, true, nil
10721206
}

‎pkg/controller/build/utils/selectors.go

+1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ func GetMachineOSBuildLabels(mosc *mcfgv1.MachineOSConfig, mcp *mcfgv1.MachineCo
1717
return map[string]string{
1818
constants.TargetMachineConfigPoolLabelKey: mcp.Name,
1919
constants.RenderedMachineConfigLabelKey: mcp.Spec.Configuration.Name,
20+
constants.RenderedMCWithImageLabel: mcp.Spec.Configuration.Name,
2021
constants.MachineOSConfigNameLabelKey: mosc.Name,
2122
}
2223
}

‎pkg/controller/common/helpers.go

+8
Original file line numberDiff line numberDiff line change
@@ -1462,3 +1462,11 @@ func GetCAsFromConfigMap(cm *corev1.ConfigMap, key string) ([]byte, error) {
14621462
}
14631463
return nil, fmt.Errorf("%s not found in %s/%s", key, cm.Namespace, cm.Name)
14641464
}
1465+
1466+
// Determines if an on-cluster layering image rollout and rebuild is required for the changes applied on the new MC
1467+
func RequiresRebuild(oldMC, newMC *mcfgv1.MachineConfig) bool {
1468+
return oldMC.Spec.OSImageURL != newMC.Spec.OSImageURL ||
1469+
oldMC.Spec.KernelType != newMC.Spec.KernelType ||
1470+
!reflect.DeepEqual(oldMC.Spec.Extensions, newMC.Spec.Extensions) ||
1471+
!reflect.DeepEqual(oldMC.Spec.KernelArguments, newMC.Spec.KernelArguments)
1472+
}

0 commit comments

Comments
 (0)