[v1.7.x] Only reset network if a network configurator is used

.obs/chartfile/elemental-operator-crds-helm/templates/crds.yaml

@@ -150,9 +150,10 @@ spec:
                       nmstate, or nmconnections formats)
                     x-kubernetes-preserve-unknown-fields: true
                   configurator:
-                    default: nmc
+                    default: none
                     description: Configurator
                     enum:
+                    - none
                     - nmc
                     - nmstate
                     - nmconnections
@@ -956,9 +957,10 @@ spec:
                           nmstate, or nmconnections formats)
                         x-kubernetes-preserve-unknown-fields: true
                       configurator:
-                        default: nmc
+                        default: none
                         description: Configurator
                         enum:
+                        - none
                         - nmc
                         - nmstate
                         - nmconnections

api/v1beta1/machineinventory_types.go

@@ -51,6 +51,7 @@ type MachineInventorySpec struct {
 	// IPAddressPools is a list of IPAddressPool associated to this machine.
 	IPAddressPools map[string]*corev1.TypedLocalObjectReference `json:"ipAddressPools,omitempty"`
 	// NetworkConfig is the final NetworkConfig.
+	// +optional
 	Network NetworkConfig `json:"network,omitempty"`
 }
 

api/v1beta1/types.go

@@ -178,8 +178,8 @@ const (
 // NetworkTemplate contains a map of IPAddressPools and a schemaless network config template.
 type NetworkTemplate struct {
 	// Configurator
-	// +kubebuilder:validation:Enum=nmc;nmstate;nmconnections
-	// +kubebuilder:default:=nmc
+	// +kubebuilder:validation:Enum=none;nmc;nmstate;nmconnections
+	// +kubebuilder:default:=none
 	Configurator string `json:"configurator,omitempty" yaml:"configurator,omitempty"`
 	// IPAddresses contains a map of IPPools references
 	IPAddresses map[string]*corev1.TypedLocalObjectReference `json:"ipAddresses,omitempty" yaml:"ipAddresses,omitempty"`
@@ -202,8 +202,8 @@ type NetworkTemplate struct {
 // can do the substitution itself.
 type NetworkConfig struct {
 	// Configurator
-	// +kubebuilder:validation:Enum=nmc;nmstate;nmconnections
-	// +kubebuilder:default:=nmc
+	// +kubebuilder:validation:Enum=none;nmc;nmstate;nmconnections
+	// +kubebuilder:default:=none
 	Configurator string `json:"configurator,omitempty" yaml:"configurator,omitempty"`
 	// IPAddresses contains a map of claimed IPAddresses
 	IPAddresses map[string]string `json:"ipAddresses,omitempty" yaml:"ipAddresses,omitempty"`

config/crd/bases/elemental.cattle.io_machineinventories.yaml

@@ -144,9 +144,10 @@ spec:
                       nmstate, or nmconnections formats)
                     x-kubernetes-preserve-unknown-fields: true
                   configurator:
-                    default: nmc
+                    default: none
                     description: Configurator
                     enum:
+                    - none
                     - nmc
                     - nmstate
                     - nmconnections

config/crd/bases/elemental.cattle.io_machineregistrations.yaml

@@ -190,9 +190,10 @@ spec:
                           nmstate, or nmconnections formats)
                         x-kubernetes-preserve-unknown-fields: true
                       configurator:
-                        default: nmc
+                        default: none
                         description: Configurator
                         enum:
+                        - none
                         - nmc
                         - nmstate
                         - nmconnections

controllers/machineinventory_controller.go

@@ -47,6 +47,7 @@ import (
 	elementalv1 "github.com/rancher/elemental-operator/api/v1beta1"
 	systemagent "github.com/rancher/elemental-operator/internal/system-agent"
 	"github.com/rancher/elemental-operator/pkg/log"
+	"github.com/rancher/elemental-operator/pkg/network"
 	"github.com/rancher/elemental-operator/pkg/util"
 )
 
@@ -267,11 +268,13 @@ func (r *MachineInventoryReconciler) updatePlanSecretWithReset(ctx context.Conte
 	var resetPlan []byte
 	var err error
 
+	networkNeedsReset := mInventory.Spec.Network.Configurator != network.ConfiguratorNone
+
 	unmanaged, unmanagedFound := mInventory.Annotations[elementalv1.MachineInventoryOSUnmanagedAnnotation]
 	if unmanagedFound && unmanaged == "true" {
 		checksum, resetPlan, err = r.newUnmanagedResetPlan(ctx)
 	} else {
-		checksum, resetPlan, err = r.newResetPlan(ctx)
+		checksum, resetPlan, err = r.newResetPlan(ctx, networkNeedsReset)
 	}
 
 	if err != nil {
@@ -396,7 +399,7 @@ func (r *MachineInventoryReconciler) reconcileNetworkConfig(ctx context.Context,
 	return ctrl.Result{}, nil
 }
 
-func (r *MachineInventoryReconciler) newResetPlan(ctx context.Context) (string, []byte, error) {
+func (r *MachineInventoryReconciler) newResetPlan(ctx context.Context, resetNetwork bool) (string, []byte, error) {
 	logger := ctrl.LoggerFrom(ctx)
 
 	logger.Info("Creating new Reset plan secret")
@@ -422,6 +425,40 @@ func (r *MachineInventoryReconciler) newResetPlan(ctx context.Context) (string,
 		return "", nil, fmt.Errorf("marshalling local reset cloud-config to yaml: %w", err)
 	}
 
+	resetInstructions := []systemagent.OneTimeInstruction{}
+
+	if resetNetwork {
+		resetInstructions = append(resetInstructions, systemagent.OneTimeInstruction{
+			CommonInstruction: systemagent.CommonInstruction{
+				Name:    "restore first boot config",
+				Command: "elemental-register",
+				Args:    []string{"--debug", "--reset-network"},
+			},
+		})
+	}
+
+	resetInstructions = append(resetInstructions, systemagent.OneTimeInstruction{
+		CommonInstruction: systemagent.CommonInstruction{
+			Name:    "configure next boot to recovery mode",
+			Command: "grub2-editenv",
+			Args: []string{
+				"/oem/grubenv",
+				"set",
+				"next_entry=recovery",
+			},
+		},
+	})
+	resetInstructions = append(resetInstructions, systemagent.OneTimeInstruction{
+		CommonInstruction: systemagent.CommonInstruction{
+			Name:    "schedule reboot",
+			Command: "shutdown",
+			Args: []string{
+				"-r",
+				"+1", // Need to have time to confirm plan execution before rebooting
+			},
+		},
+	})
+
 	// This is the remote plan that should trigger the reboot into recovery and reset
 	resetPlan := systemagent.Plan{
 		Files: []systemagent.File{
@@ -431,39 +468,7 @@ func (r *MachineInventoryReconciler) newResetPlan(ctx context.Context) (string,
 				Permissions: "0600",
 			},
 		},
-		OneTimeInstructions: []systemagent.OneTimeInstruction{
-			{
-				CommonInstruction: systemagent.CommonInstruction{
-					Name:    "restore first boot config",
-					Command: "elemental-register",
-					Args: []string{
-						"--debug",
-						"--reset-network",
-					},
-				},
-			},
-			{
-				CommonInstruction: systemagent.CommonInstruction{
-					Name:    "configure next boot to recovery mode",
-					Command: "grub2-editenv",
-					Args: []string{
-						"/oem/grubenv",
-						"set",
-						"next_entry=recovery",
-					},
-				},
-			},
-			{
-				CommonInstruction: systemagent.CommonInstruction{
-					Name:    "schedule reboot",
-					Command: "shutdown",
-					Args: []string{
-						"-r",
-						"+1", // Need to have time to confirm plan execution before rebooting
-					},
-				},
-			},
-		},
+		OneTimeInstructions: resetInstructions,
 	}
 
 	var buf bytes.Buffer

controllers/machineinventory_controller_test.go

@@ -18,6 +18,7 @@ package controllers
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"time"
 
@@ -33,6 +34,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	elementalv1 "github.com/rancher/elemental-operator/api/v1beta1"
+	systemagent "github.com/rancher/elemental-operator/internal/system-agent"
+	"github.com/rancher/elemental-operator/pkg/network"
 	"github.com/rancher/elemental-operator/pkg/test"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -227,6 +230,69 @@ var _ = Describe("createPlanSecret", func() {
 		})
 		Expect(r.createPlanSecret(ctx, mInventory)).To(Succeed())
 	})
+
+	It("should not reset network if configurator none", func() {
+		inventoryWithNoneNetwork := &elementalv1.MachineInventory{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "machine-inventory-network-none",
+				Namespace: "default",
+			},
+			Spec: elementalv1.MachineInventorySpec{
+				Network: elementalv1.NetworkConfig{
+					Configurator: network.ConfiguratorNone,
+				},
+			},
+		}
+		Expect(r.Create(ctx, inventoryWithNoneNetwork)).Should(Succeed())
+		Expect(r.createPlanSecret(ctx, inventoryWithNoneNetwork)).To(Succeed())
+		Expect(r.Get(ctx, types.NamespacedName{Namespace: inventoryWithNoneNetwork.Namespace, Name: inventoryWithNoneNetwork.Name}, planSecret)).To(Succeed())
+		Expect(r.updatePlanSecretWithReset(ctx, inventoryWithNoneNetwork, planSecret)).Should(Succeed())
+		Expect(r.Get(ctx, types.NamespacedName{Namespace: inventoryWithNoneNetwork.Namespace, Name: inventoryWithNoneNetwork.Name}, planSecret)).To(Succeed())
+		planData, found := planSecret.Data["plan"]
+		Expect(found).To(BeTrue(), "Secret should contain reset plan data")
+
+		plan := &systemagent.Plan{}
+		Expect(json.Unmarshal(planData, plan)).Should(Succeed())
+		Expect(len(plan.OneTimeInstructions)).Should(Equal(2), "Should contain 2 reset instructions")
+		Expect(plan.OneTimeInstructions).ShouldNot(ContainElement(systemagent.OneTimeInstruction{
+			CommonInstruction: systemagent.CommonInstruction{
+				Name:    "restore first boot config",
+				Command: "elemental-register",
+				Args:    []string{"--debug", "--reset-network"},
+			}}))
+		Expect(test.CleanupAndWait(ctx, cl, inventoryWithNoneNetwork)).To(Succeed())
+	})
+
+	It("should reset network if configurator not none", func() {
+		inventoryWithNetwork := &elementalv1.MachineInventory{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "machine-inventory-network-nmc",
+				Namespace: "default",
+			},
+			Spec: elementalv1.MachineInventorySpec{
+				Network: elementalv1.NetworkConfig{
+					Configurator: network.ConfiguratorNmc,
+				},
+			},
+		}
+		Expect(r.Create(ctx, inventoryWithNetwork)).Should(Succeed())
+		Expect(r.createPlanSecret(ctx, inventoryWithNetwork)).To(Succeed())
+		Expect(r.Get(ctx, types.NamespacedName{Namespace: inventoryWithNetwork.Namespace, Name: inventoryWithNetwork.Name}, planSecret)).To(Succeed())
+		Expect(r.updatePlanSecretWithReset(ctx, inventoryWithNetwork, planSecret)).Should(Succeed())
+		Expect(r.Get(ctx, types.NamespacedName{Namespace: inventoryWithNetwork.Namespace, Name: inventoryWithNetwork.Name}, planSecret)).To(Succeed())
+		planData, found := planSecret.Data["plan"]
+		Expect(found).To(BeTrue(), "Secret should contain reset plan data")
+
+		plan := &systemagent.Plan{}
+		Expect(json.Unmarshal(planData, plan)).Should(Succeed())
+		Expect(plan.OneTimeInstructions).Should(ContainElement(systemagent.OneTimeInstruction{
+			CommonInstruction: systemagent.CommonInstruction{
+				Name:    "restore first boot config",
+				Command: "elemental-register",
+				Args:    []string{"--debug", "--reset-network"},
+			}}))
+		Expect(test.CleanupAndWait(ctx, cl, inventoryWithNetwork)).To(Succeed())
+	})
 })
 
 var _ = Describe("updateInventoryWithAdoptionStatus", func() {
@@ -475,7 +541,7 @@ var _ = Describe("handle finalizer", func() {
 			Namespace: mInventory.Namespace,
 		}, mInventory)).To(Succeed())
 
-		wantChecksum, wantPlan, err := r.newResetPlan(ctx)
+		wantChecksum, wantPlan, err := r.newResetPlan(ctx, false)
 		Expect(err).ToNot(HaveOccurred())
 
 		// Check Plan status

pkg/network/network.go

@@ -29,6 +29,13 @@ import (
 	"github.com/twpayne/go-vfs"
 )
 
+const (
+	ConfiguratorNone          = "none"
+	ConfiguratorNmc           = "nmc"
+	ConfiguratorNmstate       = "nmstate"
+	ConfiguratorNmconnections = "nmconnections"
+)
+
 const (
 	// common
 	systemConnectionsDir = "/etc/NetworkManager/system-connections"
@@ -70,13 +77,17 @@ func NewConfigurator(fs vfs.FS) Configurator {
 
 func (c *configurator) GetNetworkConfigApplicator(networkConfig elementalv1.NetworkConfig) (schema.YipConfig, error) {
 	switch networkConfig.Configurator {
-	case "nmc":
+	case "":
+		return schema.YipConfig{}, nil
+	case ConfiguratorNone:
+		return schema.YipConfig{}, nil
+	case ConfiguratorNmc:
 		nc := nmcConfigurator{fs: c.fs, runner: c.runner}
 		return nc.GetNetworkConfigApplicator(networkConfig)
-	case "nmstate":
+	case ConfiguratorNmstate:
 		nc := nmstateConfigurator{fs: c.fs, runner: c.runner}
 		return nc.GetNetworkConfigApplicator(networkConfig)
-	case "nmconnections":
+	case ConfiguratorNmconnections:
 		nc := networkManagerConfigurator{fs: c.fs, runner: c.runner}
 		return nc.GetNetworkConfigApplicator(networkConfig)
 	default:
@@ -122,8 +133,7 @@ func (c *configurator) ResetNetworkConfig() error {
 	}
 
 	// Delete all .nmconnection files. This will also delete any "static" connection that the user defined in the base image for example.
-	// Which means maybe that is not supported anymore, or if we want to support it we should make sure we only delete the ones created by elemental,
-	// for example prefixing all files with "elemental-" or just parsing the network config again at this stage to determine the file names.
+	// Note that ResetNetworkConfig() is only called when the network config is Elemental driven, hence we not expect any other custom connection defined.
 	log.Debug("Deleting all .nmconnection configs")
 	if err := c.runner.Run("find", systemConnectionsDir, "-name", "*.nmconnection", "-type", "f", "-delete"); err != nil {
 		return fmt.Errorf("deleting all %s/*.nmconnection: %w", systemConnectionsDir, err)

pkg/server/api_registration.go

@@ -36,6 +36,7 @@ import (
 	elementalv1 "github.com/rancher/elemental-operator/api/v1beta1"
 	"github.com/rancher/elemental-operator/pkg/hostinfo"
 	"github.com/rancher/elemental-operator/pkg/log"
+	"github.com/rancher/elemental-operator/pkg/network"
 	"github.com/rancher/elemental-operator/pkg/register"
 	"github.com/rancher/elemental-operator/pkg/templater"
 )
@@ -125,7 +126,7 @@ func (i *InventoryServer) unauthenticatedResponse(registration *elementalv1.Mach
 		Encode(config)
 }
 
-func (i *InventoryServer) writeMachineInventoryCloudConfig(conn *websocket.Conn, protoVersion register.MessageType, inventory *elementalv1.MachineInventory, registration *elementalv1.MachineRegistration, netConf *elementalv1.NetworkConfig) error {
+func (i *InventoryServer) writeMachineInventoryCloudConfig(conn *websocket.Conn, protoVersion register.MessageType, inventory *elementalv1.MachineInventory, registration *elementalv1.MachineRegistration, netConf elementalv1.NetworkConfig) error {
 	sa := &corev1.ServiceAccount{}
 
 	if err := i.Get(i, types.NamespacedName{
@@ -195,7 +196,7 @@ func (i *InventoryServer) writeMachineInventoryCloudConfig(conn *websocket.Conn,
 		return err
 	}
 
-	if netConf != nil {
+	if netConf.Configurator != network.ConfiguratorNone && netConf.Configurator != "" {
 		netData, err := yaml.Marshal(netConf)
 		if err != nil {
 			log.Errorf("error marshalling network config: %v", err)
@@ -321,19 +322,19 @@ func (i *InventoryServer) handleUpdate(conn *websocket.Conn, protoVersion regist
 	return nil
 }
 
-func (i *InventoryServer) handleGetNetworkConfig(inventory *elementalv1.MachineInventory) (*elementalv1.NetworkConfig, error) {
+func (i *InventoryServer) handleGetNetworkConfig(inventory *elementalv1.MachineInventory) (elementalv1.NetworkConfig, error) {
 	ctx, cancel := context.WithTimeout(context.TODO(), register.RegistrationDeadlineSeconds*time.Second)
 	defer cancel()
 
 	for {
 		select {
 		case <-ctx.Done():
-			return nil, fmt.Errorf("NewtworkConfig not ready")
+			return elementalv1.NetworkConfig{}, fmt.Errorf("NewtworkConfig not ready")
 		default:
 			time.Sleep(time.Second)
 		}
 		if err := i.Get(ctx, client.ObjectKeyFromObject(inventory), inventory); err != nil {
-			return nil, fmt.Errorf("getting machine inventory: %w", err)
+			return elementalv1.NetworkConfig{}, fmt.Errorf("getting machine inventory: %w", err)
 		}
 		conditionFound := false
 		for _, condition := range inventory.Status.Conditions {
@@ -352,12 +353,12 @@ func (i *InventoryServer) handleGetNetworkConfig(inventory *elementalv1.MachineI
 		break
 	}
 
-	return &inventory.Spec.Network, nil
+	return inventory.Spec.Network, nil
 }
 
 func (i *InventoryServer) handleGet(conn *websocket.Conn, protoVersion register.MessageType, inventory *elementalv1.MachineInventory, registration *elementalv1.MachineRegistration) error {
 	var err error
-	var netConf *elementalv1.NetworkConfig
+	var netConf elementalv1.NetworkConfig
 
 	inventory, err = i.commitMachineInventory(inventory)
 	if err != nil {