From a583ab5f5af46ea5116580084627cd5e690b0783 Mon Sep 17 00:00:00 2001 From: Anusha Hegde Date: Tue, 26 Apr 2022 09:41:13 +0530 Subject: [PATCH] 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 | 1 + agent/host_agent_test.go | 58 ++++++++++++++++++- agent/main.go | 35 ++++++----- agent/reconciler/host_reconciler.go | 25 +++++--- agent/reconciler/reconciler_test.go | 54 +++++++++++++++-- .../v1beta1/condition_consts.go | 5 ++ 6 files changed, 148 insertions(+), 30 deletions(-) diff --git a/agent/help_flag_test.go b/agent/help_flag_test.go index 1ec6fe413..5995db626 100644 --- a/agent/help_flag_test.go +++ b/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", diff --git a/agent/host_agent_test.go b/agent/host_agent_test.go index 01f9e2f10..538f97283 100644 --- a/agent/host_agent_test.go +++ b/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()) + }) + }) }) diff --git a/agent/main.go b/agent/main.go index dda892d08..5f4711fb9 100644 --- a/agent/main.go +++ b/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 { diff --git a/agent/reconciler/host_reconciler.go b/agent/reconciler/host_reconciler.go index 7ab83ae37..4a30cbd72 100644 --- a/agent/reconciler/host_reconciler.go +++ b/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 diff --git a/agent/reconciler/reconciler_test.go b/agent/reconciler/reconciler_test.go index d9e5d7ea6..af5bd15c4 100644 --- a/agent/reconciler/reconciler_test.go +++ b/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,6 +258,7 @@ 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", @@ -232,6 +266,7 @@ runCmd: }) 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 }) }) }) diff --git a/apis/infrastructure/v1beta1/condition_consts.go b/apis/infrastructure/v1beta1/condition_consts.go index 9b8b37e05..9cdf1ab3a 100644 --- a/apis/infrastructure/v1beta1/condition_consts.go +++ b/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