WIP: Pick last non-empty OSImageURL

docs/MachineConfiguration.md

@@ -98,4 +98,4 @@ Ignition config keys as well).
 
 The operating system used to first boot a machine is platform dependent. For example, on AWS AMIs are used to bring up EC2Instances. But for day-2 updates of the cluster, the MachineConfigDaemon uses the `OSImageURL` to fetch new operating system during updates. An example for OSImageURL is `quay.io/openshift/$CONTAINER@sha256:$DIGEST`. The digest is required to ensure there are no race conditions.
 
-When combining multiple MachineConfig objects, OSImageURL field is ignored from all the MachineConfig objects except the one defined by Openshift.
+When combining multiple MachineConfig objects, the last non-empty OSImageURL field is used. In general though, this is only expected to be set by the release payload and not cluster administrators.

pkg/apis/machineconfiguration.openshift.io/v1/helpers.go

@@ -11,7 +11,7 @@ import (
 // MergeMachineConfigs combines multiple machineconfig objects into one object.
 // It sorts all the configs in increasing order of their name.
 // It uses the Ign config from first object as base and appends all the rest.
-// It only uses the OSImageURL from first object and ignores it from rest.
+// The last non-empty OSImageURL will be chosen.
 func MergeMachineConfigs(configs []*MachineConfig) *MachineConfig {
 	if len(configs) == 0 {
 		return nil
@@ -21,6 +21,10 @@ func MergeMachineConfigs(configs []*MachineConfig) *MachineConfig {
 	outOSImageURL := configs[0].Spec.OSImageURL
 	outIgn := configs[0].Spec.Config
 	for idx := 1; idx < len(configs); idx++ {
+		osImageURL := configs[idx].Spec.OSImageURL
+		if osImageURL != "" {
+			outOSImageURL = osImageURL
+		}
 		outIgn = ignv2_2.Append(outIgn, configs[idx].Spec.Config)
 	}
 

pkg/apis/machineconfiguration.openshift.io/v1/helpers_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 var (
@@ -139,3 +140,47 @@ func TestRemoveMachineConfigPoolCondition(t *testing.T) {
 		})
 	}
 }
+
+func TestMergeMachineConfigs(t *testing.T) {
+	var configs []*MachineConfig
+	configs = append(configs, &MachineConfig{
+		ObjectMeta: metav1.ObjectMeta{Name: "4"},
+		Spec: MachineConfigSpec{
+			OSImageURL: "",
+		},
+	})
+
+	targetOSImageURL := "pivot://example.com/os@sha256:thetarget"
+	configs = append(configs, &MachineConfig{
+		ObjectMeta: metav1.ObjectMeta{Name: "3"},
+		Spec: MachineConfigSpec{
+			OSImageURL: targetOSImageURL,
+		},
+	})
+
+	configs = append(configs, &MachineConfig{
+		ObjectMeta: metav1.ObjectMeta{Name: "0"},
+		Spec: MachineConfigSpec{
+			OSImageURL: "",
+		},
+	})
+
+	configs = append(configs, &MachineConfig{
+		ObjectMeta: metav1.ObjectMeta{Name: "1"},
+		Spec: MachineConfigSpec{
+			OSImageURL:"pivot://example.com/os@sha256:notthetarget",
+		},
+	})
+
+	configs = append(configs, &MachineConfig{
+		ObjectMeta: metav1.ObjectMeta{Name: "2"},
+		Spec: MachineConfigSpec{
+			OSImageURL: "",
+		},
+	})
+
+	merged := MergeMachineConfigs(configs)
+	if merged.Spec.OSImageURL != targetOSImageURL {
+		t.Errorf("OSImageURL expected: %s, received: %s", targetOSImageURL, merged.Spec.OSImageURL)
+	}
+}