Merge pull request #239 from BarthV/none-mode

Add sentinel bootstrap file check modes in CRD
kubernetes-sigs · May 3, 2023 · b49257e · b49257e
2 parents 0cd24c7 + 9a7fe21
commit b49257e
Show file tree

Hide file tree

Showing 15 changed files with 188 additions and 3 deletions.
diff --git a/api/v1alpha1/kubevirtmachine_types.go b/api/v1alpha1/kubevirtmachine_types.go
@@ -47,12 +47,27 @@ type KubevirtMachineSpec struct {
 	// +optional
 	ProviderID *string `json:"providerID,omitempty"`
 
+	// BootstrapCheckSpec defines how the CAPK controller is checking CAPI Sentinel file inside the VM.
+	// +optional
+	BootstrapCheckSpec VirtualMachineBootstrapCheckSpec `json:"virtualMachineBootstrapCheck,omitempty"`
+
 	// InfraClusterSecretRef is a reference to a secret with a kubeconfig for external cluster used for infra.
 	// When nil, this defaults to the value present in the KubevirtCluster object's spec associated with this machine.
 	// +optional
 	InfraClusterSecretRef *corev1.ObjectReference `json:"infraClusterSecretRef,omitempty"`
 }
 
+// VirtualMachineBootstrapCheckSpec defines how the controller will remotely check CAPI Sentinel file content.
+type VirtualMachineBootstrapCheckSpec struct {
+	// CheckStrategy describes how CAPK controller will validate a successful CAPI bootstrap.
+	// Following specified method, CAPK will try to retrieve the state of the CAPI Sentinel file from the VM.
+	// Possible values are: "none" or "ssh" (default is "ssh") and this value is validated by apiserver.
+	// +optional
+	// +kubebuilder:validation:Enum=none;ssh
+	// +kubebuilder:default:=ssh
+	CheckStrategy string `json:"checkStrategy,omitempty"`
+}
+
 // KubevirtMachineStatus defines the observed state of KubevirtMachine.
 type KubevirtMachineStatus struct {
 	// Ready denotes that the machine is ready

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtmachines.yaml
@@ -80,6 +80,22 @@ spec:
               providerID:
                 description: ProviderID TBD what to use for Kubevirt
                 type: string
+              virtualMachineBootstrapCheck:
+                description: BootstrapCheckSpec defines how the CAPK controller is
+                  checking CAPI Sentinel file inside the VM.
+                properties:
+                  checkStrategy:
+                    default: ssh
+                    description: 'CheckStrategy describes how CAPK controller will
+                      validate a successful CAPI bootstrap. Following specified method,
+                      CAPK will try to retrieve the state of the CAPI Sentinel file
+                      from the VM. Possible values are: "none" or "ssh" (default is
+                      "ssh") and this value is validated by apiserver.'
+                    enum:
+                    - none
+                    - ssh
+                    type: string
+                type: object
               virtualMachineTemplate:
                 description: VirtualMachineTemplateSpec defines the desired state
                   of the kubevirt VM.

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtmachinetemplates.yaml
@@ -91,6 +91,23 @@ spec:
                       providerID:
                         description: ProviderID TBD what to use for Kubevirt
                         type: string
+                      virtualMachineBootstrapCheck:
+                        description: BootstrapCheckSpec defines how the CAPK controller
+                          is checking CAPI Sentinel file inside the VM.
+                        properties:
+                          checkStrategy:
+                            default: ssh
+                            description: 'CheckStrategy describes how CAPK controller
+                              will validate a successful CAPI bootstrap. Following
+                              specified method, CAPK will try to retrieve the state
+                              of the CAPI Sentinel file from the VM. Possible values
+                              are: "none" or "ssh" (default is "ssh") and this value
+                              is validated by apiserver.'
+                            enum:
+                            - none
+                            - ssh
+                            type: string
+                        type: object
                       virtualMachineTemplate:
                         description: VirtualMachineTemplateSpec defines the desired
                           state of the kubevirt VM.

diff --git a/pkg/kubevirt/machine.go b/pkg/kubevirt/machine.go
@@ -226,6 +226,25 @@ func (m *Machine) SupportsCheckingIsBootstrapped() bool {
 
 // IsBootstrapped checks if the VM is bootstrapped with Kubernetes.
 func (m *Machine) IsBootstrapped() bool {
+	// CheckStrategy value is already sanitized by apiserver
+	switch m.machineContext.KubevirtMachine.Spec.BootstrapCheckSpec.CheckStrategy {
+	case "none":
+		// skip bootstrap check and always returns positively
+		return true
+
+	case "":
+		fallthrough // ssh is default check strategy, fallthrough
+	case "ssh":
+		return m.IsBootstrappedWithSSH()
+
+	default:
+		// Since CRD CheckStrategy field is validated by an enum, this case should never be hit
+		return false
+	}
+}
+
+// IsBootstrappedWithSSH checks if the VM is bootstrapped with Kubernetes using SSH strategy.
+func (m *Machine) IsBootstrappedWithSSH() bool {
 	if !m.IsReady() || m.sshKeys == nil {
 		return false
 	}

diff --git a/pkg/kubevirt/machine_test.go b/pkg/kubevirt/machine_test.go
@@ -31,6 +31,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
+	"sigs.k8s.io/cluster-api-provider-kubevirt/api/v1alpha1"
 	"sigs.k8s.io/cluster-api-provider-kubevirt/pkg/context"
 	"sigs.k8s.io/cluster-api-provider-kubevirt/pkg/ssh"
 	"sigs.k8s.io/cluster-api-provider-kubevirt/pkg/testing"
@@ -64,6 +65,8 @@ var _ = Describe("Without KubeVirt VM running", func() {
 	virtualMachine := testing.NewVirtualMachine(virtualMachineInstance)
 
 	BeforeEach(func() {
+		kubevirtMachine.Spec.BootstrapCheckSpec = v1alpha1.VirtualMachineBootstrapCheckSpec{}
+
 		machineContext = &context.MachineContext{
 			Context:             gocontext.TODO(),
 			Cluster:             cluster,
@@ -114,8 +117,29 @@ var _ = Describe("Without KubeVirt VM running", func() {
 		Expect(externalMachine.IsReady()).To(BeFalse())
 	})
 
-	It("IsBootstrapped should return false", func() {
+	It("default mode: IsBootstrapped should return false", func() {
+		externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte{})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(externalMachine.IsBootstrapped()).To(BeFalse())
+	})
+
+	It("ssh mode: IsBootstrapped return false", func() {
+		externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte{})
+		externalMachine.machineContext.KubevirtMachine.Spec.BootstrapCheckSpec.CheckStrategy = "ssh"
+		Expect(err).NotTo(HaveOccurred())
+		Expect(externalMachine.IsBootstrapped()).To(BeFalse())
+	})
+
+	It("none mode: IsBootstrapped should be forced to be true", func() {
+		externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte{})
+		externalMachine.machineContext.KubevirtMachine.Spec.BootstrapCheckSpec.CheckStrategy = "none"
+		Expect(err).NotTo(HaveOccurred())
+		Expect(externalMachine.IsBootstrapped()).To(BeTrue())
+	})
+
+	It("invalid mode: IsBootstrapped should return false", func() {
 		externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte{})
+		externalMachine.machineContext.KubevirtMachine.Spec.BootstrapCheckSpec.CheckStrategy = "impossible-invalid-input"
 		Expect(err).NotTo(HaveOccurred())
 		Expect(externalMachine.IsBootstrapped()).To(BeFalse())
 	})
@@ -176,6 +200,8 @@ var _ = Describe("With KubeVirt VM running", func() {
 	virtualMachine := testing.NewVirtualMachine(virtualMachineInstance)
 
 	BeforeEach(func() {
+		kubevirtMachine.Spec.BootstrapCheckSpec = v1alpha1.VirtualMachineBootstrapCheckSpec{}
+
 		machineContext = &context.MachineContext{
 			Context:             gocontext.TODO(),
 			Cluster:             cluster,
@@ -234,12 +260,33 @@ var _ = Describe("With KubeVirt VM running", func() {
 		Expect(externalMachine.IsReady()).To(BeTrue())
 	})
 
-	It("IsBootstrapped should return true", func() {
+	It("default mode: IsBootstrapped should return true", func() {
+		externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte(sshKey))
+		Expect(err).NotTo(HaveOccurred())
+		Expect(externalMachine.IsBootstrapped()).To(BeTrue())
+	})
+
+	It("ssh mode: IsBootstrapped return true", func() {
+		externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte(sshKey))
+		externalMachine.machineContext.KubevirtMachine.Spec.BootstrapCheckSpec.CheckStrategy = "ssh"
+		Expect(err).NotTo(HaveOccurred())
+		Expect(externalMachine.IsBootstrapped()).To(BeTrue())
+	})
+
+	It("none mode: IsBootstrapped should be forced to be true", func() {
 		externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte(sshKey))
+		externalMachine.machineContext.KubevirtMachine.Spec.BootstrapCheckSpec.CheckStrategy = "none"
 		Expect(err).NotTo(HaveOccurred())
 		Expect(externalMachine.IsBootstrapped()).To(BeTrue())
 	})
 
+	It("invalid mode: IsBootstrapped should return false", func() {
+		externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte(sshKey))
+		externalMachine.machineContext.KubevirtMachine.Spec.BootstrapCheckSpec.CheckStrategy = "impossible-invalid-input"
+		Expect(err).NotTo(HaveOccurred())
+		Expect(externalMachine.IsBootstrapped()).To(BeFalse())
+	})
+
 	It("SupportsCheckingIsBootstrapped should return true", func() {
 		externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte(sshKey))
 		Expect(err).NotTo(HaveOccurred())
@@ -319,6 +366,8 @@ var _ = Describe("With KubeVirt VM running externally", func() {
 	virtualMachine := testing.NewVirtualMachine(virtualMachineInstance)
 
 	BeforeEach(func() {
+		kubevirtMachine.Spec.BootstrapCheckSpec = v1alpha1.VirtualMachineBootstrapCheckSpec{}
+
 		machineContext = &context.MachineContext{
 			Context:             gocontext.TODO(),
 			Cluster:             cluster,
@@ -377,12 +426,33 @@ var _ = Describe("With KubeVirt VM running externally", func() {
 		Expect(externalMachine.IsReady()).To(BeTrue())
 	})
 
-	It("IsBootstrapped should return true", func() {
+	It("default mode: IsBootstrapped should return true", func() {
+		externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte(sshKey))
+		Expect(err).NotTo(HaveOccurred())
+		Expect(externalMachine.IsBootstrapped()).To(BeTrue())
+	})
+
+	It("ssh mode: IsBootstrapped return true", func() {
+		externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte(sshKey))
+		externalMachine.machineContext.KubevirtMachine.Spec.BootstrapCheckSpec.CheckStrategy = "ssh"
+		Expect(err).NotTo(HaveOccurred())
+		Expect(externalMachine.IsBootstrapped()).To(BeTrue())
+	})
+
+	It("none mode: IsBootstrapped should be forced to be true", func() {
 		externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte(sshKey))
+		externalMachine.machineContext.KubevirtMachine.Spec.BootstrapCheckSpec.CheckStrategy = "none"
 		Expect(err).NotTo(HaveOccurred())
 		Expect(externalMachine.IsBootstrapped()).To(BeTrue())
 	})
 
+	It("invalid mode: IsBootstrapped should return false", func() {
+		externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte(sshKey))
+		externalMachine.machineContext.KubevirtMachine.Spec.BootstrapCheckSpec.CheckStrategy = "impossible-invalid-input"
+		Expect(err).NotTo(HaveOccurred())
+		Expect(externalMachine.IsBootstrapped()).To(BeFalse())
+	})
+
 	It("SupportsCheckingIsBootstrapped should return true", func() {
 		externalMachine, err := defaultTestMachine(machineContext, namespace, fakeClient, fakeVMCommandExecutor, []byte(sshKey))
 		Expect(err).NotTo(HaveOccurred())

diff --git a/pkg/testing/common.go b/pkg/testing/common.go
@@ -68,6 +68,8 @@ func NewKubevirtMachine(kubevirtMachineName, machineName string) *infrav1.Kubevi
 					Template: &kubevirtv1.VirtualMachineInstanceTemplateSpec{},
 				},
 			},
+
+			BootstrapCheckSpec: infrav1.VirtualMachineBootstrapCheckSpec{},
 		},
 		Status: infrav1.KubevirtMachineStatus{},
 	}

diff --git a/templates/cluster-template-ext-infra.yaml b/templates/cluster-template-ext-infra.yaml
@@ -100,6 +100,8 @@ metadata:
 spec:
   template:
     spec:
+      virtualMachineBootstrapCheck:
+        checkStrategy: ssh
       virtualMachineTemplate:
         spec:
           runStrategy: Always

diff --git a/templates/cluster-template-kccm.yaml b/templates/cluster-template-kccm.yaml
@@ -41,6 +41,8 @@ metadata:
 spec:
   template:
     spec:
+      virtualMachineBootstrapCheck:
+        checkStrategy: ssh
       virtualMachineTemplate:
         metadata:
           namespace: "${NAMESPACE}"
@@ -100,6 +102,8 @@ metadata:
 spec:
   template:
     spec:
+      virtualMachineBootstrapCheck:
+        checkStrategy: ssh
       virtualMachineTemplate:
         metadata:
           namespace: "${NAMESPACE}"

diff --git a/templates/cluster-template-lb-kccm.yaml b/templates/cluster-template-lb-kccm.yaml
@@ -41,6 +41,8 @@ metadata:
 spec:
   template:
     spec:
+      virtualMachineBootstrapCheck:
+        checkStrategy: ssh
       virtualMachineTemplate:
         metadata:
           namespace: "${NAMESPACE}"
@@ -100,6 +102,8 @@ metadata:
 spec:
   template:
     spec:
+      virtualMachineBootstrapCheck:
+        checkStrategy: ssh
       virtualMachineTemplate:
         metadata:
           namespace: "${NAMESPACE}"

diff --git a/templates/cluster-template-lb.yaml b/templates/cluster-template-lb.yaml
@@ -41,6 +41,8 @@ metadata:
 spec:
   template:
     spec:
+      virtualMachineBootstrapCheck:
+        checkStrategy: ssh
       virtualMachineTemplate:
         metadata:
           namespace: "${NAMESPACE}"
@@ -100,6 +102,8 @@ metadata:
 spec:
   template:
     spec:
+      virtualMachineBootstrapCheck:
+        checkStrategy: ssh
       virtualMachineTemplate:
         metadata:
           namespace: "${NAMESPACE}"

diff --git a/templates/cluster-template-passt-kccm.yaml b/templates/cluster-template-passt-kccm.yaml
@@ -41,6 +41,8 @@ metadata:
 spec:
   template:
     spec:
+      virtualMachineBootstrapCheck:
+        checkStrategy: ssh
       virtualMachineTemplate:
         metadata:
           namespace: "${NAMESPACE}"
@@ -105,6 +107,8 @@ metadata:
 spec:
   template:
     spec:
+      virtualMachineBootstrapCheck:
+        checkStrategy: ssh
       virtualMachineTemplate:
         metadata:
           namespace: "${NAMESPACE}"

diff --git a/templates/cluster-template-persistent-storage-kccm.yaml b/templates/cluster-template-persistent-storage-kccm.yaml
@@ -37,6 +37,8 @@ metadata:
 spec:
   template:
     spec:
+      virtualMachineBootstrapCheck:
+        checkStrategy: ssh
       virtualMachineTemplate:
         metadata:
           namespace: "${NAMESPACE}"
@@ -110,6 +112,8 @@ metadata:
 spec:
   template:
     spec:
+      virtualMachineBootstrapCheck:
+        checkStrategy: ssh
       virtualMachineTemplate:
         metadata:
           namespace: "${NAMESPACE}"

diff --git a/templates/cluster-template-persistent-storage.yaml b/templates/cluster-template-persistent-storage.yaml
@@ -37,6 +37,8 @@ metadata:
 spec:
   template:
     spec:
+      virtualMachineBootstrapCheck:
+        checkStrategy: ssh
       virtualMachineTemplate:
         metadata:
           namespace: "${NAMESPACE}"
@@ -110,6 +112,8 @@ metadata:
 spec:
   template:
     spec:
+      virtualMachineBootstrapCheck:
+        checkStrategy: ssh
       virtualMachineTemplate:
         metadata:
           namespace: "${NAMESPACE}"