Skip to content

Commit c0b6461

Browse files
isabella-janssenisabella-janssen
committed
controller, daemon, and upgrade monitor: update MachineConfigNode update flow to include image mode status reporting fields
1 parent 9fc0b0a commit c0b6461

File tree

4 files changed

+276
-3
lines changed

4 files changed

+276
-3
lines changed

‎pkg/controller/common/layered_node_state.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ func (l *LayeredNodeState) SetDesiredStateFromPool(mcp *mcfgv1.MachineConfigPool
141141
node := l.Node()
142142

143143
metav1.SetMetaDataAnnotation(&node.ObjectMeta, daemonconsts.DesiredMachineConfigAnnotationKey, mcp.Spec.Configuration.Name)
144+
// todo: check if a new desired config actually deletes an image anno
144145
delete(node.Annotations, daemonconsts.DesiredImageAnnotationKey)
145146

146147
l.node = node
@@ -319,3 +320,29 @@ func (l *LayeredNodeState) CheckNodeCandidacyForUpdate(layered bool, pool *mcfgv
319320

320321
return true
321322
}
323+
324+
// Returns the desired config version annotation present on the node.
325+
func (l *LayeredNodeState) GetDesiredConfigAnnotation() string {
326+
return l.node.Annotations[daemonconsts.DesiredMachineConfigAnnotationKey]
327+
}
328+
329+
// Returns the desired image annotation present on the node.
330+
func (l *LayeredNodeState) GetDesiredImageAnnotation() string {
331+
return l.node.Annotations[daemonconsts.DesiredImageAnnotationKey]
332+
}
333+
334+
// GetDesiredAnnotationsFromMachineOSConfig gets the desired config version and desired image values from the associated MOSC and MOSB
335+
func (l *LayeredNodeState) GetDesiredAnnotationsFromMachineConfigPool(mcp *mcfgv1.MachineConfigPool) (desriedConfig string) {
336+
return mcp.Spec.Configuration.Name
337+
}
338+
339+
// GetDesiredAnnotationsFromMachineOSConfig gets the desired config version and desired image values from the associated MOSC and MOSB
340+
func (l *LayeredNodeState) GetDesiredAnnotationsFromMachineOSConfig(mosc *mcfgv1.MachineOSConfig, mosb *mcfgv1.MachineOSBuild) (desriedConfig, desiredImage string) {
341+
desiredImage = ""
342+
moscs := NewMachineOSConfigState(mosc)
343+
if moscs.HasOSImage() {
344+
desiredImage = moscs.GetOSImage()
345+
}
346+
347+
return mosb.Spec.MachineConfig.Name, desiredImage
348+
}

‎pkg/controller/node/node_controller.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"time"
1010

1111
helpers "github.com/openshift/machine-config-operator/pkg/helpers"
12+
"github.com/openshift/machine-config-operator/pkg/upgrademonitor"
1213

1314
configv1 "github.com/openshift/api/config/v1"
1415
features "github.com/openshift/api/features"
@@ -1288,11 +1289,21 @@ func (ctrl *Controller) updateCandidateNode(mosc *mcfgv1.MachineOSConfig, mosb *
12881289
}
12891290

12901291
lns := ctrlcommon.NewLayeredNodeState(oldNode)
1292+
desiredConfig := lns.GetDesiredConfigAnnotation()
1293+
desiredImage := lns.GetDesiredImageAnnotation()
12911294
if !layered {
1295+
// TODO: see if this can be consolidated
12921296
lns.SetDesiredStateFromPool(pool)
1297+
// TODO: see what's up with the desired image annotation in this case...
1298+
desiredConfig = lns.GetDesiredAnnotationsFromMachineConfigPool(pool)
12931299
} else {
1300+
// TODO: see if this can be consolidated
12941301
lns.SetDesiredStateFromMachineOSConfig(mosc, mosb)
1302+
// TODO: test this
1303+
desiredConfig, desiredImage = lns.GetDesiredAnnotationsFromMachineOSConfig(mosc, mosb)
12951304
}
1305+
// TODO: maybe add a try/catch for the error handling if the MCN does not yest exist
1306+
upgrademonitor.UpdateMachineConfigNodeSpecDesiredAnnotations(ctrl.fgHandler, ctrl.client, nodeName, desiredConfig, desiredImage)
12961307

12971308
// Set the desired state to match the pool.
12981309

‎pkg/daemon/update.go‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2572,6 +2572,26 @@ func (dn *Daemon) updateLayeredOS(config *mcfgv1.MachineConfig) error {
25722572
return fmt.Errorf("failed to update OS from local storage: %s: %w", newURL, err)
25732573
}
25742574
} else {
2575+
// Report ImagePulledFromRegistry condition as unknown (pulling)
2576+
if dn.fgHandler != nil && dn.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) && dn.node != nil && dn.mcfgClient != nil {
2577+
pool, poolErr := helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, dn.node)
2578+
if poolErr == nil {
2579+
err := upgrademonitor.GenerateAndApplyMachineConfigNodes(
2580+
&upgrademonitor.Condition{State: mcfgv1.MachineConfigNodeImagePulledFromRegistry, Reason: string(mcfgv1.MachineConfigNodeImagePulledFromRegistry), Message: fmt.Sprintf("Pulling OS image %s from registry", newURL)},
2581+
nil,
2582+
metav1.ConditionUnknown,
2583+
metav1.ConditionFalse,
2584+
dn.node,
2585+
dn.mcfgClient,
2586+
dn.fgHandler,
2587+
pool,
2588+
)
2589+
if err != nil {
2590+
klog.Errorf("Error setting ImagePulledFromRegistry condition to unknown: %v", err)
2591+
}
2592+
}
2593+
}
2594+
25752595
// Workaround for OCPBUGS-43406, retry the remote rebase with backoff,
25762596
// such that if we happen to update while the CoreDNS pod is being restarted,
25772597
// the next retry should succeed if no other issues are present.
@@ -2588,8 +2608,47 @@ func (dn *Daemon) updateLayeredOS(config *mcfgv1.MachineConfig) error {
25882608
}
25892609
return true, nil
25902610
}); err != nil {
2611+
// Report ImagePulledFromRegistry condition as false (failed)
2612+
if dn.fgHandler != nil && dn.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) && dn.node != nil && dn.mcfgClient != nil {
2613+
pool, poolErr := helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, dn.node)
2614+
if poolErr == nil {
2615+
condErr := upgrademonitor.GenerateAndApplyMachineConfigNodes(
2616+
&upgrademonitor.Condition{State: mcfgv1.MachineConfigNodeImagePulledFromRegistry, Reason: string(mcfgv1.MachineConfigNodeImagePulledFromRegistry), Message: fmt.Sprintf("Failed to pull OS image %s from registry: %v", newURL, err)},
2617+
nil,
2618+
metav1.ConditionFalse,
2619+
metav1.ConditionFalse,
2620+
dn.node,
2621+
dn.mcfgClient,
2622+
dn.fgHandler,
2623+
pool,
2624+
)
2625+
if condErr != nil {
2626+
klog.Errorf("Error setting ImagePulledFromRegistry condition to false: %v", condErr)
2627+
}
2628+
}
2629+
}
25912630
return fmt.Errorf("Failed to update OS to %s after retries: %w", newURL, err)
25922631
}
2632+
2633+
// Report ImagePulledFromRegistry condition as true (success)
2634+
if dn.fgHandler != nil && dn.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) && dn.node != nil && dn.mcfgClient != nil {
2635+
pool, poolErr := helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, dn.node)
2636+
if poolErr == nil {
2637+
err := upgrademonitor.GenerateAndApplyMachineConfigNodes(
2638+
&upgrademonitor.Condition{State: mcfgv1.MachineConfigNodeImagePulledFromRegistry, Reason: string(mcfgv1.MachineConfigNodeImagePulledFromRegistry), Message: fmt.Sprintf("Successfully pulled OS image %s from registry", newURL)},
2639+
nil,
2640+
metav1.ConditionTrue,
2641+
metav1.ConditionFalse,
2642+
dn.node,
2643+
dn.mcfgClient,
2644+
dn.fgHandler,
2645+
pool,
2646+
)
2647+
if err != nil {
2648+
klog.Errorf("Error setting ImagePulledFromRegistry condition to true: %v", err)
2649+
}
2650+
}
2651+
}
25932652
}
25942653

25952654
return nil
@@ -2742,9 +2801,70 @@ func (dn *CoreOSDaemon) applyLayeredOSChanges(mcDiff machineConfigDiff, oldConfi
27422801
// TODO(jkyros): the original intent was that we use the extensions container as a service, but that currently results
27432802
// in a lot of complexity due to boostrap and firstboot where the service isn't easily available, so for now we are going
27442803
// to extract them to disk like we did previously.
2804+
2805+
// Report ImagePulledFromRegistry condition as unknown (pulling)
2806+
if dn.fgHandler != nil && dn.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) && dn.node != nil && dn.mcfgClient != nil {
2807+
pool, poolErr := helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, dn.node)
2808+
if poolErr == nil {
2809+
err := upgrademonitor.GenerateAndApplyMachineConfigNodes(
2810+
&upgrademonitor.Condition{State: mcfgv1.MachineConfigNodeImagePulledFromRegistry, Reason: string(mcfgv1.MachineConfigNodeImagePulledFromRegistry), Message: fmt.Sprintf("Pulling image %s from registry", newConfig.Spec.BaseOSExtensionsContainerImage)},
2811+
nil,
2812+
metav1.ConditionUnknown,
2813+
metav1.ConditionFalse,
2814+
dn.node,
2815+
dn.mcfgClient,
2816+
dn.fgHandler,
2817+
pool,
2818+
)
2819+
if err != nil {
2820+
klog.Errorf("Error setting ImagePulledFromRegistry condition to unknown: %v", err)
2821+
}
2822+
}
2823+
}
2824+
27452825
if osExtensionsContentDir, err = ExtractExtensionsImage(newConfig.Spec.BaseOSExtensionsContainerImage); err != nil {
2826+
// Report ImagePulledFromRegistry condition as false (failed)
2827+
if dn.fgHandler != nil && dn.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) && dn.node != nil && dn.mcfgClient != nil {
2828+
pool, poolErr := helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, dn.node)
2829+
if poolErr == nil {
2830+
condErr := upgrademonitor.GenerateAndApplyMachineConfigNodes(
2831+
&upgrademonitor.Condition{State: mcfgv1.MachineConfigNodeImagePulledFromRegistry, Reason: string(mcfgv1.MachineConfigNodeImagePulledFromRegistry), Message: fmt.Sprintf("Failed to pull image %s from registry: %v", newConfig.Spec.BaseOSExtensionsContainerImage, err)},
2832+
nil,
2833+
metav1.ConditionFalse,
2834+
metav1.ConditionFalse,
2835+
dn.node,
2836+
dn.mcfgClient,
2837+
dn.fgHandler,
2838+
pool,
2839+
)
2840+
if condErr != nil {
2841+
klog.Errorf("Error setting ImagePulledFromRegistry condition to false: %v", condErr)
2842+
}
2843+
}
2844+
}
27462845
return err
27472846
}
2847+
2848+
// Report ImagePulledFromRegistry condition as true (success)
2849+
if dn.fgHandler != nil && dn.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) && dn.node != nil && dn.mcfgClient != nil {
2850+
pool, poolErr := helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, dn.node)
2851+
if poolErr == nil {
2852+
err := upgrademonitor.GenerateAndApplyMachineConfigNodes(
2853+
&upgrademonitor.Condition{State: mcfgv1.MachineConfigNodeImagePulledFromRegistry, Reason: string(mcfgv1.MachineConfigNodeImagePulledFromRegistry), Message: fmt.Sprintf("Successfully pulled image %s from registry", newConfig.Spec.BaseOSExtensionsContainerImage)},
2854+
nil,
2855+
metav1.ConditionTrue,
2856+
metav1.ConditionFalse,
2857+
dn.node,
2858+
dn.mcfgClient,
2859+
dn.fgHandler,
2860+
pool,
2861+
)
2862+
if err != nil {
2863+
klog.Errorf("Error setting ImagePulledFromRegistry condition to true: %v", err)
2864+
}
2865+
}
2866+
}
2867+
27482868
// Delete extracted OS image once we are done.
27492869
defer os.RemoveAll(osExtensionsContentDir)
27502870

‎pkg/upgrademonitor/upgrade_monitor.go‎

Lines changed: 118 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ func generateAndApplyMachineConfigNodes(
135135

136136
// we use this array to see if the MCN has all of its conditions set
137137
// if not we set a sane default
138+
// TODO: decide if the image reg stuff should be added here too
138139
allConditionTypes := []mcfgv1.StateProgress{
139140
mcfgv1.MachineConfigNodeUpdatePrepared,
140141
mcfgv1.MachineConfigNodeUpdateExecuted,
@@ -217,8 +218,20 @@ func generateAndApplyMachineConfigNodes(
217218

218219
case condition.Status != metav1.ConditionFalse && reset:
219220
condition.Status = metav1.ConditionFalse
220-
condition.Message = fmt.Sprintf("Action during update to %s: %s", newMCNode.Spec.ConfigVersion.Desired, condition.Message)
221221
condition.LastTransitionTime = metav1.Now()
222+
223+
// TODO: test this in the enable and disable case
224+
// Set the update to annotation to the desired rendered MC version by default for the condition message
225+
updateToMessage := fmt.Sprintf("Action during update to %s: %s", newMCNode.Spec.ConfigVersion.Desired, condition.Message)
226+
// Handle OCL update cases differently
227+
if newMCNode.Spec.ConfigImage.DesiredImage != newMCNode.Status.ConfigImage.CurrentImage {
228+
if newMCNode.Spec.ConfigImage.DesiredImage != "" { // Handle case when desired image exists
229+
updateToMessage = fmt.Sprintf("Action during update to %s: %s", newMCNode.Spec.ConfigImage.DesiredImage, condition.Message)
230+
} else { // When the desired image is empty, it means OCL is being disabled; provide a more useful message in this case
231+
updateToMessage = fmt.Sprintf("Action during update to disable image mode: %s", condition.Message)
232+
}
233+
}
234+
condition.Message = updateToMessage
222235
}
223236
condition.DeepCopyInto(&newMCNode.Status.Conditions[i])
224237
}
@@ -250,6 +263,27 @@ func generateAndApplyMachineConfigNodes(
250263
newMCNode.Status.ConfigVersion.Current = node.Annotations[daemonconsts.CurrentMachineConfigAnnotationKey]
251264
}
252265

266+
// Set current and desired image values in MCN.Status.ConfigImage
267+
// This is only done when the ImageModeStatusReporting feature gate is enabled
268+
if fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) {
269+
// TODO: test if this flow accurately updates the current image annotaiton on OCL disable
270+
newMCNStatusConfigImage := mcfgv1.MachineConfigNodeStatusConfigImage{}
271+
currentImageAnnotation := node.Annotations[daemonconsts.CurrentImageAnnotationKey]
272+
desiredImageAnnotation := node.Annotations[daemonconsts.DesiredImageAnnotationKey]
273+
274+
// Set current image if annotation exists
275+
if currentImageAnnotation != "" {
276+
newMCNStatusConfigImage.CurrentImage = mcfgv1.ImageDigestFormat(currentImageAnnotation)
277+
}
278+
279+
// Set desired image if annotation exists
280+
if desiredImageAnnotation != "" {
281+
newMCNStatusConfigImage.DesiredImage = mcfgv1.ImageDigestFormat(desiredImageAnnotation)
282+
}
283+
284+
newMCNode.Status.ConfigImage = newMCNStatusConfigImage
285+
}
286+
253287
// if we do not need a new MCN, generate the apply configurations for this object
254288
if !needNewMCNode {
255289
statusconfigVersionApplyConfig := machineconfigurationv1.MachineConfigNodeStatusMachineConfigVersion().WithDesired(newMCNode.Status.ConfigVersion.Desired)
@@ -262,6 +296,23 @@ func generateAndApplyMachineConfigNodes(
262296
WithObservedGeneration(newMCNode.Generation + 1).
263297
WithConfigVersion(statusconfigVersionApplyConfig)
264298

299+
// Add ConfigImage to apply configuration if feature gate is enabled and image annotations exist
300+
if fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) && (newMCNode.Status.ConfigImage.CurrentImage != "" || newMCNode.Status.ConfigImage.DesiredImage != "") {
301+
configImageApplyConfig := machineconfigurationv1.MachineConfigNodeStatusConfigImage()
302+
303+
// Set current image if it exists
304+
if newMCNode.Status.ConfigImage.CurrentImage != "" {
305+
configImageApplyConfig = configImageApplyConfig.WithCurrentImage(newMCNode.Status.ConfigImage.CurrentImage)
306+
}
307+
308+
// Set desired image if it exists
309+
if newMCNode.Status.ConfigImage.DesiredImage != "" {
310+
configImageApplyConfig = configImageApplyConfig.WithDesiredImage(newMCNode.Status.ConfigImage.DesiredImage)
311+
}
312+
313+
statusApplyConfig = statusApplyConfig.WithConfigImage(configImageApplyConfig)
314+
}
315+
265316
if fgHandler.Enabled(features.FeatureGatePinnedImages) {
266317
if imageSetApplyConfig == nil {
267318
for _, imageSet := range newMCNode.Status.PinnedImageSets {
@@ -335,8 +386,64 @@ func isSingletonCondition(singletonConditionTypes []mcfgv1.StateProgress, condit
335386
return false
336387
}
337388

389+
// func UpdateMachineConfigNodeSpecDesiredImage(fgHandler ctrlcommon.FeatureGatesHandler, node *corev1.Node, mcfgClient mcfgclientset.Interface) error {
390+
func UpdateMachineConfigNodeSpecDesiredAnnotations(fgHandler ctrlcommon.FeatureGatesHandler, mcfgClient mcfgclientset.Interface, nodeName string, desiredConfig string, desiredImage string) error {
391+
if fgHandler == nil {
392+
return nil
393+
}
394+
395+
// Check that the MachineConfigNode and ImageModeStatusReporting feature gates are enabled
396+
if !fgHandler.Enabled(features.FeatureGateMachineConfigNodes) || !fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) {
397+
klog.Infof("MachineConfigNode FeatureGate is not enabled.")
398+
return nil
399+
}
400+
401+
if !fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) {
402+
klog.Infof("ImageModeStatusReporting FeatureGate is not enabled. Please enable the TechPreviewNoUpgrade FeatureSet to use ImageModeStatusReporting.")
403+
return nil
404+
}
405+
406+
// get the existing MCN
407+
mcn, mcnErr := mcfgClient.MachineconfigurationV1().
408+
MachineConfigNodes().
409+
Get(context.TODO(), nodeName, metav1.GetOptions{})
410+
// // TODO: figure out the need of this check
411+
// if needNewMCNode {
412+
// return fmt.Errorf("No MachineConfigNode exists yet for node %v. Use `GenerateAndApplyMachineConfigNodeSpec` instead.", node)
413+
// }
414+
if mcnErr != nil {
415+
// no existing MCN found since no resource found
416+
if apierrors.IsNotFound(mcnErr) {
417+
return fmt.Errorf("MCN for %s node does not exits. Skipping MCN spec update.", nodeName)
418+
}
419+
return mcnErr
420+
}
421+
422+
// Set the desired config annotation
423+
mcn.Spec.ConfigVersion.Desired = NotYetSet
424+
if desiredConfig != "" {
425+
mcn.Spec.ConfigVersion.Desired = desiredConfig
426+
}
427+
428+
// Set the desired image annotation
429+
// TODO: figure out if this actually handles the removal of the field
430+
mcn.Spec.ConfigImage = mcfgv1.MachineConfigNodeSpecConfigImage{}
431+
if desiredImage != "" {
432+
mcn.Spec.ConfigImage = mcfgv1.MachineConfigNodeSpecConfigImage{
433+
DesiredImage: mcfgv1.ImageDigestFormat(desiredImage),
434+
}
435+
}
436+
437+
if _, err := mcfgClient.MachineconfigurationV1().MachineConfigNodes().Update(context.TODO(), mcn, metav1.UpdateOptions{FieldManager: "machine-config-operator"}); err != nil {
438+
return fmt.Errorf("failed to update the %s mcn spec with the new desired config and image value: %w",
439+
nodeName, err)
440+
}
441+
return nil
442+
}
443+
338444
// GenerateAndApplyMachineConfigNodeSpec generates and applies a new MCN spec based off the node state
339445
func GenerateAndApplyMachineConfigNodeSpec(fgHandler ctrlcommon.FeatureGatesHandler, pool string, node *corev1.Node, mcfgClient mcfgclientset.Interface) error {
446+
// TODO: add in this function something to handle addig the desired image annotation if it exists
340447
if fgHandler == nil || node == nil {
341448
return nil
342449
}
@@ -348,7 +455,7 @@ func GenerateAndApplyMachineConfigNodeSpec(fgHandler ctrlcommon.FeatureGatesHand
348455
// get the existing MCN, or if it DNE create one below
349456
mcNode, needNewMCNode := createOrGetMachineConfigNode(mcfgClient, node)
350457
newMCNode := mcNode.DeepCopy()
351-
// set the spec config version
458+
// Set the MCN owner references
352459
newMCNode.ObjectMeta.OwnerReferences = []metav1.OwnerReference{
353460
{
354461
APIVersion: "v1",
@@ -358,14 +465,22 @@ func GenerateAndApplyMachineConfigNodeSpec(fgHandler ctrlcommon.FeatureGatesHand
358465
},
359466
}
360467

468+
// Set the desired config version in the MCN
361469
newMCNode.Spec.ConfigVersion = mcfgv1.MachineConfigNodeSpecMachineConfigVersion{
362470
Desired: node.Annotations[daemonconsts.DesiredMachineConfigAnnotationKey],
363471
}
364-
// Set desired config to NotYetSet if the annotation is empty to satisfy API validation
472+
// If the desired config does not yet exist for the node, the desired config should be set to NotYetSet
365473
if newMCNode.Spec.ConfigVersion.Desired == "" {
366474
newMCNode.Spec.ConfigVersion.Desired = NotYetSet
367475
}
368476

477+
// Set the desired image in the MCN if it exists
478+
newMCNode.Spec.ConfigImage = mcfgv1.MachineConfigNodeSpecConfigImage{}
479+
if node.Annotations[daemonconsts.DesiredImageAnnotationKey] != "" {
480+
newMCNode.Spec.ConfigImage.DesiredImage = mcfgv1.ImageDigestFormat(node.Annotations[daemonconsts.DesiredImageAnnotationKey])
481+
}
482+
483+
// Set the MCN pool and node names
369484
newMCNode.Spec.Pool = mcfgv1.MCOObjectReference{
370485
Name: pool,
371486
}

0 commit comments

Comments
 (0)