Adding use-installer-flag to host agent (#502)

* Adding use-installer-flag to host agent

- this flag indicates we have to use the script provided by the
  installer controller
- refactored skip-installation code path
- added unit and integration tests cases

agent/help_flag_test.go

@@ -23,6 +23,7 @@ var _ = Describe("Help flag for host agent", func() {
 				"--metricsbindaddress string",
 				"--namespace string",
 				"--skip-installation",
+				"--use-installer-controller",
 				"--version",
 				"-v, --v",
 				"--feature-gates mapStringBool",

agent/host_agent_test.go

@@ -507,8 +507,9 @@ var _ = Describe("Agent", func() {
 
 		BeforeEach(func() {
 			ns = builder.Namespace("testns").Build()
-			Expect(k8sClient.Create(context.TODO(), ns)).NotTo(HaveOccurred(), "failed to create test namespace")
 			ctx = context.TODO()
+			Expect(k8sClient.Create(ctx, ns)).NotTo(HaveOccurred(), "failed to create test namespace")
+
 			var err error
 			hostName, err = os.Hostname()
 			Expect(err).NotTo(HaveOccurred())
@@ -561,4 +562,59 @@ var _ = Describe("Agent", func() {
 		})
 
 	})
+
+	Context("When the host agent is executed with --use-installer-controller flag", func() {
+		var (
+			ns               *corev1.Namespace
+			ctx              context.Context
+			err              error
+			hostName         string
+			runner           *e2e.ByoHostRunner
+			byoHostContainer *container.ContainerCreateCreatedBody
+			output           dockertypes.HijackedResponse
+		)
+
+		BeforeEach(func() {
+			ns = builder.Namespace("testns").Build()
+			ctx = context.TODO()
+			Expect(k8sClient.Create(ctx, ns)).NotTo(HaveOccurred(), "failed to create test namespace")
+
+			hostName, err = os.Hostname()
+			Expect(err).NotTo(HaveOccurred())
+
+			runner = setupTestInfra(ctx, hostName, getKubeConfig().Name(), ns)
+			runner.CommandArgs["--use-installer-controller"] = ""
+
+			byoHostContainer, err = runner.SetupByoDockerHost()
+			Expect(err).NotTo(HaveOccurred())
+
+		})
+
+		AfterEach(func() {
+			cleanup(runner.Context, byoHostContainer, ns, agentLogFile)
+		})
+
+		It("should not call the intree installer", func() {
+			output, _, err = runner.ExecByoDockerHost(byoHostContainer)
+			Expect(err).NotTo(HaveOccurred())
+			defer output.Close()
+			f := e2e.WriteDockerLog(output, agentLogFile)
+			defer func() {
+				deferredErr := f.Close()
+				if deferredErr != nil {
+					e2e.Showf("error closing file %s: %v", agentLogFile, deferredErr)
+				}
+			}()
+			Eventually(func() (done bool) {
+				_, err := os.Stat(agentLogFile)
+				if err == nil {
+					data, err := os.ReadFile(agentLogFile)
+					if err == nil && strings.Contains(string(data), "\"msg\"=\"use-installer-controller flag set, skipping intree installer\"") {
+						return true
+					}
+				}
+				return false
+			}, 30).Should(BeTrue())
+		})
+	})
 })

agent/main.go

@@ -86,6 +86,7 @@ func setupflags() {
 	flag.StringVar(&metricsbindaddress, "metricsbindaddress", ":8080", "metricsbindaddress is the TCP address that the controller should bind to for serving prometheus metrics.It can be set to \"0\" to disable the metrics serving")
 	flag.StringVar(&downloadpath, "downloadpath", "/var/lib/byoh/bundles", "File System path to keep the downloads")
 	flag.BoolVar(&skipInstallation, "skip-installation", false, "If you want to skip installation of the kubernetes component binaries")
+	flag.BoolVar(&useInstallerController, "use-installer-controller", false, "If you want to skip the intree installer and use the default or your own installer controller")
 	flag.BoolVar(&printVersion, "version", false, "Print the version of the agent")
 
 	pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
@@ -124,14 +125,15 @@ func setupTemplateParser() *cloudinit.TemplateParser {
 }
 
 var (
-	namespace          string
-	scheme             *runtime.Scheme
-	labels             = make(labelFlags)
-	metricsbindaddress string
-	downloadpath       string
-	skipInstallation   bool
-	printVersion       bool
-	k8sInstaller       reconciler.IK8sInstaller
+	namespace              string
+	scheme                 *runtime.Scheme
+	labels                 = make(labelFlags)
+	metricsbindaddress     string
+	downloadpath           string
+	skipInstallation       bool
+	useInstallerController bool
+	printVersion           bool
+	k8sInstaller           reconciler.IK8sInstaller
 )
 
 // TODO - fix logging
@@ -197,8 +199,9 @@ func main() {
 	}
 
 	if skipInstallation {
-		k8sInstaller = nil
 		logger.Info("skip-installation flag set, skipping installer initialisation")
+	} else if useInstallerController {
+		logger.Info("use-installer-controller flag set, skipping intree installer")
 	} else {
 		// increasing installer log level to 1, so that it wont be logged by default
 		k8sInstaller, err = installer.New(downloadpath, installer.BundleTypeK8s, logger.V(1))
@@ -208,12 +211,14 @@ func main() {
 	}
 
 	hostReconciler := &reconciler.HostReconciler{
-		Client:         k8sClient,
-		CmdRunner:      cloudinit.CmdRunner{},
-		FileWriter:     cloudinit.FileWriter{},
-		TemplateParser: setupTemplateParser(),
-		Recorder:       mgr.GetEventRecorderFor("hostagent-controller"),
-		K8sInstaller:   k8sInstaller,
+		Client:                 k8sClient,
+		CmdRunner:              cloudinit.CmdRunner{},
+		FileWriter:             cloudinit.FileWriter{},
+		TemplateParser:         setupTemplateParser(),
+		Recorder:               mgr.GetEventRecorderFor("hostagent-controller"),
+		K8sInstaller:           k8sInstaller,
+		SkipK8sInstallation:    skipInstallation,
+		UseInstallerController: useInstallerController,
 	}
 
 	if err = hostReconciler.SetupWithManager(context.TODO(), mgr); err != nil {

agent/reconciler/host_reconciler.go

@@ -37,12 +37,14 @@ type IK8sInstaller interface {
 
 // HostReconciler encapsulates the data/logic needed to reconcile a ByoHost
 type HostReconciler struct {
-	Client         client.Client
-	CmdRunner      cloudinit.ICmdRunner
-	FileWriter     cloudinit.IFileWriter
-	TemplateParser cloudinit.ITemplateParser
-	Recorder       record.EventRecorder
-	K8sInstaller   IK8sInstaller
+	Client                 client.Client
+	CmdRunner              cloudinit.ICmdRunner
+	FileWriter             cloudinit.IFileWriter
+	TemplateParser         cloudinit.ITemplateParser
+	Recorder               record.EventRecorder
+	K8sInstaller           IK8sInstaller
+	SkipK8sInstallation    bool
+	UseInstallerController bool
 }
 
 const (
@@ -112,8 +114,14 @@ func (r *HostReconciler) reconcileNormal(ctx context.Context, byoHost *infrastru
 			return ctrl.Result{}, err
 		}
 
-		if r.K8sInstaller == nil {
+		if r.SkipK8sInstallation {
 			logger.Info("Skipping installation of k8s components")
+		} else if r.UseInstallerController {
+			if byoHost.Spec.InstallationSecret == nil {
+				logger.Info("K8sInstallationSecret not ready")
+				conditions.MarkFalse(byoHost, infrastructurev1beta1.K8sNodeBootstrapSucceeded, infrastructurev1beta1.K8sInstallationSecretUnavailableReason, clusterv1.ConditionSeverityInfo, "")
+				return ctrl.Result{}, nil
+			}
 		} else {
 			err = r.installK8sComponents(ctx, byoHost)
 			if err != nil {
@@ -205,7 +213,7 @@ func (r *HostReconciler) hostCleanUp(ctx context.Context, byoHost *infrastructur
 		if err != nil {
 			return err
 		}
-		if r.K8sInstaller == nil {
+		if r.SkipK8sInstallation {
 			logger.Info("Skipping uninstallation of k8s components")
 		} else {
 			err = r.uninstallk8sComponents(ctx, byoHost)
@@ -263,6 +271,7 @@ func (r *HostReconciler) installK8sComponents(ctx context.Context, byoHost *infr
 	bundleRegistry := byoHost.GetAnnotations()[infrastructurev1beta1.BundleLookupBaseRegistryAnnotation]
 	k8sVersion := byoHost.GetAnnotations()[infrastructurev1beta1.K8sVersionAnnotation]
 	byohBundleTag := byoHost.GetAnnotations()[infrastructurev1beta1.BundleLookupTagAnnotation]
+
 	err := r.K8sInstaller.Install(bundleRegistry, k8sVersion, byohBundleTag)
 	if err != nil {
 		return err

agent/reconciler/reconciler_test.go

@@ -45,12 +45,14 @@ var _ = Describe("Byohost Agent Tests", func() {
 		fakeInstaller = &reconcilerfakes.FakeIK8sInstaller{}
 		recorder = record.NewFakeRecorder(32)
 		hostReconciler = &reconciler.HostReconciler{
-			Client:         k8sClient,
-			CmdRunner:      fakeCommandRunner,
-			FileWriter:     fakeFileWriter,
-			TemplateParser: fakeTemplateParser,
-			Recorder:       recorder,
-			K8sInstaller:   nil,
+			Client:                 k8sClient,
+			CmdRunner:              fakeCommandRunner,
+			FileWriter:             fakeFileWriter,
+			TemplateParser:         fakeTemplateParser,
+			Recorder:               recorder,
+			K8sInstaller:           nil,
+			SkipK8sInstallation:    false,
+			UseInstallerController: false,
 		}
 	})
 
@@ -177,6 +179,36 @@ runCmd:
 					Expect(patchHelper.Patch(ctx, byoHost, patch.WithStatusObservedGeneration{})).NotTo(HaveOccurred())
 				})
 
+				Context("When use-installer-controller is set", func() {
+					BeforeEach(func() {
+						hostReconciler.UseInstallerController = true
+					})
+
+					It("should set the Reason to InstallationSecretUnavailableReason", func() {
+						result, reconcilerErr := hostReconciler.Reconcile(ctx, controllerruntime.Request{
+							NamespacedName: byoHostLookupKey,
+						})
+						Expect(result).To(Equal(controllerruntime.Result{}))
+						Expect(reconcilerErr).ToNot(HaveOccurred())
+
+						updatedByoHost := &infrastructurev1beta1.ByoHost{}
+						err := k8sClient.Get(ctx, byoHostLookupKey, updatedByoHost)
+						Expect(err).ToNot(HaveOccurred())
+
+						byoHostRegistrationSucceeded := conditions.Get(updatedByoHost, infrastructurev1beta1.K8sNodeBootstrapSucceeded)
+						Expect(*byoHostRegistrationSucceeded).To(conditions.MatchCondition(clusterv1.Condition{
+							Type:     infrastructurev1beta1.K8sNodeBootstrapSucceeded,
+							Status:   corev1.ConditionFalse,
+							Reason:   infrastructurev1beta1.K8sInstallationSecretUnavailableReason,
+							Severity: clusterv1.ConditionSeverityInfo,
+						}))
+					})
+
+					AfterEach(func() {
+						hostReconciler.UseInstallerController = false
+					})
+				})
+
 				It("should set K8sComponentsInstallationSucceeded to false with Reason K8sComponentsInstallationFailedReason if Install fails", func() {
 					hostReconciler.K8sInstaller = fakeInstaller
 					fakeInstaller.InstallReturns(errors.New("k8s components install failed"))
@@ -201,6 +233,7 @@ runCmd:
 				})
 
 				It("should set K8sNodeBootstrapSucceeded to false with Reason CloudInitExecutionFailedReason if the bootstrap execution fails", func() {
+					hostReconciler.K8sInstaller = fakeInstaller
 					fakeCommandRunner.RunCmdReturns(errors.New("I failed"))
 
 					result, reconcilerErr := hostReconciler.Reconcile(ctx, controllerruntime.Request{
@@ -225,13 +258,15 @@ runCmd:
 					// assert events
 					events := eventutils.CollectEvents(recorder.Events)
 					Expect(events).Should(ConsistOf([]string{
+						"Normal k8sComponentInstalled Successfully Installed K8s components",
 						"Warning BootstrapK8sNodeFailed k8s Node Bootstrap failed",
 						// TODO: improve test to remove this event
 						"Warning ResetK8sNodeFailed k8s Node Reset failed",
 					}))
 				})
 
 				It("should set K8sNodeBootstrapSucceeded to True if the boostrap execution succeeds", func() {
+					hostReconciler.K8sInstaller = fakeInstaller
 					result, reconcilerErr := hostReconciler.Reconcile(ctx, controllerruntime.Request{
 						NamespacedName: byoHostLookupKey,
 					})
@@ -254,11 +289,13 @@ runCmd:
 					// assert events
 					events := eventutils.CollectEvents(recorder.Events)
 					Expect(events).Should(ConsistOf([]string{
+						"Normal k8sComponentInstalled Successfully Installed K8s components",
 						"Normal BootstrapK8sNodeSucceeded k8s Node Bootstraped",
 					}))
 				})
 
 				It("should skip k8s installation if skip-installation is set", func() {
+					hostReconciler.SkipK8sInstallation = true
 					result, reconcilerErr := hostReconciler.Reconcile(ctx, controllerruntime.Request{
 						NamespacedName: byoHostLookupKey,
 					})
@@ -283,6 +320,7 @@ runCmd:
 				})
 
 				It("should execute bootstrap secret only once ", func() {
+					hostReconciler.K8sInstaller = fakeInstaller
 					_, reconcilerErr := hostReconciler.Reconcile(ctx, controllerruntime.Request{
 						NamespacedName: byoHostLookupKey,
 					})
@@ -299,6 +337,7 @@ runCmd:
 
 				AfterEach(func() {
 					Expect(k8sClient.Delete(ctx, bootstrapSecret)).NotTo(HaveOccurred())
+					hostReconciler.SkipK8sInstallation = false
 				})
 			})
 
@@ -349,6 +388,7 @@ runCmd:
 			})
 
 			It("should reset the node and set the Reason to K8sNodeAbsentReason", func() {
+				hostReconciler.K8sInstaller = fakeInstaller
 				result, reconcilerErr := hostReconciler.Reconcile(ctx, controllerruntime.Request{
 					NamespacedName: byoHostLookupKey,
 				})
@@ -386,6 +426,7 @@ runCmd:
 			})
 
 			It("should skip uninstallation if skip-installation flag is set", func() {
+				hostReconciler.SkipK8sInstallation = true
 				result, reconcilerErr := hostReconciler.Reconcile(ctx, controllerruntime.Request{
 					NamespacedName: byoHostLookupKey,
 				})
@@ -455,6 +496,7 @@ runCmd:
 
 		AfterEach(func() {
 			Expect(k8sClient.Delete(ctx, byoHost)).NotTo(HaveOccurred())
+			hostReconciler.SkipK8sInstallation = false
 		})
 	})
 })

apis/infrastructure/v1beta1/condition_consts.go

@@ -26,6 +26,11 @@ const (
 	// This secret is available on byohost.Spec.BootstrapSecret field
 	BootstrapDataSecretUnavailableReason = "BootstrapDataSecretUnavailable"
 
+	// K8sInstallationSecretUnavailableReason indicates that the installer controller is yet to provide the
+	// secret that contains installation and uninstallation scripts
+	// This secret is available on byohost.Spec.K8sInstallationSecret field
+	K8sInstallationSecretUnavailableReason = "K8sInstallationSecretUnavailable"
+
 	// CleanK8sDirectoriesFailedReason indicates that clean k8s directories failed for some reason, please
 	// delete it manually for reconcile to proceed.
 	// The cleaned directories are /run/kubeadm and /etc/cni/net.d