Refactored Host Agent, Installer Integration

agent/host_agent_test.go

@@ -187,17 +187,4 @@ var _ = Describe("Agent", func() {
 			Consistently(session.Err, "10s").ShouldNot(gbytes.Say(byoHost.Name))
 		})
 	})
-	Context("When the host agent skips k8s installation", func() {
-		It("should inform the operator that we will not install k8s", func() {
-			ns := builder.Namespace("testns").Build()
-			Expect(k8sClient.Create(context.TODO(), ns)).NotTo(HaveOccurred(), "failed to create test namespace")
-
-			command := exec.Command(pathToHostAgentBinary, "--kubeconfig", kubeconfigFile.Name(), "--namespace", ns.Name, "--skip-installation")
-
-			session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter)
-			Expect(err).NotTo(HaveOccurred())
-
-			Eventually(session.Err, "5s").Should(gbytes.Say("Skipping installation of k8s components"))
-		})
-	})
 })

agent/installer/cli.go

@@ -106,23 +106,23 @@ func runInstaller(install bool) {
 	var err error
 	if *osFlag != "" {
 		// Override current OS detection
-		i, err = newUnchecked(*osFlag, *cachePathFlag, klogger, &logPrinter{klogger})
+		i, err = newUnchecked(*osFlag, *bundleRepoFlag, *cachePathFlag, klogger, &logPrinter{klogger})
 		if err != nil {
 			klogger.Error(err, "unable to create installer")
 			return
 		}
 	} else {
-		i, err = New(*cachePathFlag, klogger)
+		i, err = New(*bundleRepoFlag, *cachePathFlag, klogger)
 		if err != nil {
 			klogger.Error(err, "unable to create installer")
 			return
 		}
 	}
 
 	if install {
-		err = i.Install(*bundleRepoFlag, *k8sFlag, *tagFlag)
+		err = i.Install(*k8sFlag, *tagFlag)
 	} else {
-		err = i.Uninstall(*bundleRepoFlag, *k8sFlag, *tagFlag)
+		err = i.Uninstall(*k8sFlag, *tagFlag)
 	}
 	if err != nil {
 		klogger.Error(err, "error installing/uninstalling")

agent/installer/installer.go

@@ -32,7 +32,7 @@ type installer struct {
 }
 
 // getSupportedRegistry returns a registry with installers for the supported OS and K8s
-func getSupportedRegistry(ob algo.OutputBuilder) registry {
+func getSupportedRegistry(bd *bundleDownloader, ob algo.OutputBuilder) registry {
 	reg := newRegistry()
 
 	addBundleInstaller := func(osBundle, k8sBundle string, stepProvider algo.K8sStepProvider) {
@@ -49,7 +49,7 @@ func getSupportedRegistry(ob algo.OutputBuilder) registry {
 
 		// BYOH Bundle Repository. Associate bundle with installer
 		linuxDistro := "Ubuntu_20.04.1_x86-64"
-		addBundleInstaller(linuxDistro, "v1.22.1", &algo.Ubuntu20_4K8s1_22{})
+		addBundleInstaller(linuxDistro, "v1.22.3", &algo.Ubuntu20_4K8s1_22{})
 		/*
 		 * PLACEHOLDER - ADD MORE K8S VERSIONS HERE
 		 */
@@ -88,7 +88,10 @@ func (bd *bundleDownloader) DownloadOrPreview(os, k8s, tag string) error {
 // New returns an installer that downloads bundles for the current OS from OCI repository with
 // address bundleRepo and stores them under downloadPath. Download path is created,
 // if it does not exist.
-func New(downloadPath string, logger logr.Logger) (*installer, error) {
+func New(bundleRepo, downloadPath string, logger logr.Logger) (*installer, error) {
+	if bundleRepo == "" {
+		return nil, fmt.Errorf("empty bundle repo")
+	}
 	if downloadPath == "" {
 		return nil, fmt.Errorf("empty download path")
 	}
@@ -100,28 +103,29 @@ func New(downloadPath string, logger logr.Logger) (*installer, error) {
 		return nil, ErrDetectOs
 	}
 
-	return newUnchecked(os, downloadPath, logger, &logPrinter{logger})
+	return newUnchecked(os, bundleRepo, downloadPath, logger, &logPrinter{logger})
 }
 
 // newUnchecked returns an installer bypassing os detection and checks of bundleRepo and downloadPath.
 // If they are empty, returned installer will runs in preview mode, i.e.
 // executes everything except the actual commands.
-func newUnchecked(currentOs, downloadPath string, logger logr.Logger, outputBuilder algo.OutputBuilder) (*installer, error) {
-	reg := getSupportedRegistry(outputBuilder)
+func newUnchecked(currentOs, bundleRepo, downloadPath string, logger logr.Logger, outputBuilder algo.OutputBuilder) (*installer, error) {
+	bd := bundleDownloader{repoAddr: bundleRepo, downloadPath: downloadPath, logger: logger}
+
+	reg := getSupportedRegistry(&bd, outputBuilder)
 	if len(reg.ListK8s(currentOs)) == 0 {
 		return nil, ErrOsK8sNotSupported
 	}
 
 	return &installer{
-		algoRegistry: reg,
-		detectedOs:   currentOs,
-		logger:       logger}, nil
+		algoRegistry:     reg,
+		bundleDownloader: bd,
+		detectedOs:       currentOs,
+		logger:           logger}, nil
 }
 
 // Install installs the specified k8s version on the current OS
-func (i *installer) Install(bundleRepo, k8sVer, tag string) error {
-	i.bundleDownloader = bundleDownloader{repoAddr: bundleRepo, downloadPath: i.downloadPath, logger: i.logger}
-
+func (i *installer) Install(k8sVer, tag string) error {
 	algoInst, err := i.getAlgoInstallerWithBundle(k8sVer, tag)
 	if err != nil {
 		return err
@@ -135,9 +139,7 @@ func (i *installer) Install(bundleRepo, k8sVer, tag string) error {
 }
 
 // Uninstal uninstalls the specified k8s version on the current OS
-func (i *installer) Uninstall(bundleRepo, k8sVer, tag string) error {
-	i.bundleDownloader = bundleDownloader{repoAddr: bundleRepo, downloadPath: i.downloadPath, logger: i.logger}
-
+func (i *installer) Uninstall(k8sVer, tag string) error {
 	algoInst, err := i.getAlgoInstallerWithBundle(k8sVer, tag)
 	if err != nil {
 		return err
@@ -187,15 +189,15 @@ func ListSupportedK8s(os string) []string {
 // getSupportedRegistryDescription returns a description registry of supported OS and k8s.
 // It that can only by queried for OS and k8s but cannot be used for install/uninstall.
 func getSupportedRegistryDescription() registry {
-	return getSupportedRegistry(nil)
+	return getSupportedRegistry(nil, nil)
 }
 
 // PreviewChanges describes the changes to install and uninstall K8s on OS without actually applying them.
 // It returns the install and uninstall changes
 // Can be invoked on a non-supported OS
 func PreviewChanges(os, k8sVer string) (install, uninstall string, err error) {
 	stepPreviewer := stringPrinter{msgFmt: "# %s"}
-	reg := getSupportedRegistry(&stepPreviewer)
+	reg := getSupportedRegistry(&bundleDownloader{}, &stepPreviewer)
 	installer := reg.GetInstaller(os, k8sVer)
 
 	if installer == nil {

agent/installer/installer_test.go

@@ -15,19 +15,19 @@ var _ = Describe("Byohost Installer Tests", func() {
 
 	Context("When installer is created for unsupported OS", func() {
 		It("Should return error", func() {
-			_, err := New("downloadPath", logr.Discard())
+			_, err := New("repo", "downloadPath", logr.Discard())
 			Expect(err).Should((HaveOccurred()))
 		})
 	})
 	Context("When installer is created with empty bundle repo", func() {
 		It("Should return error", func() {
-			_, err := New("downloadPath", logr.Discard())
+			_, err := New("", "downloadPath", logr.Discard())
 			Expect(err).Should((HaveOccurred()))
 		})
 	})
 	Context("When installer is created with empty download path", func() {
 		It("Should return error", func() {
-			_, err := New("", logr.Discard())
+			_, err := New("repo", "", logr.Discard())
 			Expect(err).Should((HaveOccurred()))
 		})
 	})
@@ -37,10 +37,10 @@ var _ = Describe("Byohost Installer Tests", func() {
 			for _, os := range osList {
 				i := NewPreviewInstaller(os, nil)
 
-				err := i.Install("repo", "unsupported-k8s", testTag)
+				err := i.Install("unsupported-k8s", testTag)
 				Expect(err).Should((HaveOccurred()))
 
-				err = i.Uninstall("repo", "unsupported-k8s", testTag)
+				err = i.Uninstall("unsupported-k8s", testTag)
 				Expect(err).Should((HaveOccurred()))
 			}
 		})
@@ -53,15 +53,15 @@ var _ = Describe("Byohost Installer Tests", func() {
 					{
 						ob := algo.OutputBuilderCounter{}
 						i := NewPreviewInstaller(os, &ob)
-						err := i.Install("repo", k8s, testTag)
+						err := i.Install(k8s, testTag)
 						Expect(err).ShouldNot((HaveOccurred()))
 						Expect(ob.LogCalledCnt).Should(Equal(24))
 					}
 
 					{
 						ob := algo.OutputBuilderCounter{}
 						i := NewPreviewInstaller(os, &ob)
-						err := i.Uninstall("repo", k8s, testTag)
+						err := i.Uninstall(k8s, testTag)
 						Expect(err).ShouldNot((HaveOccurred()))
 						Expect(ob.LogCalledCnt).Should(Equal(24))
 					}
@@ -119,7 +119,7 @@ var _ = Describe("Byohost Installer Tests", func() {
 })
 
 func NewPreviewInstaller(os string, ob algo.OutputBuilder) *installer {
-	i, err := newUnchecked(os, "", logr.Discard(), ob)
+	i, err := newUnchecked(os, "", "", logr.Discard(), ob)
 	if err != nil {
 		panic(err)
 	}

agent/main.go

@@ -11,7 +11,6 @@ import (
 	"strings"
 
 	"github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/cloudinit"
-	"github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/installer"
 	"github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/reconciler"
 	"github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/registration"
 	infrastructurev1beta1 "github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/apis/infrastructure/v1beta1"
@@ -143,18 +142,6 @@ func main() {
 		return
 	}
 
-	var k8sInstaller reconciler.Installer
-
-	if skipInstallation {
-		logger.Info("Skipping installation of k8s components")
-	} else {
-		k8sInstaller, err = installer.New(downloadpath, logger)
-		if err != nil {
-			logger.Error(err, "unable instantiate installer")
-			return
-		}
-	}
-
 	hostReconciler := &reconciler.HostReconciler{
 		Client:     k8sClient,
 		CmdRunner:  cloudinit.CmdRunner{},
@@ -164,8 +151,9 @@ func main() {
 				DefaultNetworkInterfaceName: registration.LocalHostRegistrar.ByoHostInfo.DefaultNetworkInterfaceName,
 			},
 		},
-		K8sInstaller: k8sInstaller,
-		Recorder:     mgr.GetEventRecorderFor("hostagent-controller"),
+		SkipInstallation: skipInstallation,
+		DownloadPath:     downloadpath,
+		Recorder:         mgr.GetEventRecorderFor("hostagent-controller"),
 	}
 	if err = hostReconciler.SetupWithManager(context.TODO(), mgr); err != nil {
 		logger.Error(err, "unable to create controller")

agent/reconciler/host_reconciler.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/cloudinit"
+	"github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/installer"
 	"github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/registration"
 	"github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/common"
 	corev1 "k8s.io/api/core/v1"
@@ -28,26 +29,20 @@ import (
 )
 
 type HostReconciler struct {
-	Client         client.Client
-	CmdRunner      cloudinit.ICmdRunner
-	FileWriter     cloudinit.IFileWriter
-	TemplateParser cloudinit.ITemplateParser
-	K8sInstaller   Installer
-	Recorder       record.EventRecorder
+	Client           client.Client
+	CmdRunner        cloudinit.ICmdRunner
+	FileWriter       cloudinit.IFileWriter
+	TemplateParser   cloudinit.ITemplateParser
+	Recorder         record.EventRecorder
+	SkipInstallation bool
+	DownloadPath     string
 }
 
 const (
 	bootstrapSentinelFile = "/run/cluster-api/bootstrap-success.complete"
 	KubeadmResetCommand   = "kubeadm reset --force"
 )
 
-//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate
-//counterfeiter:generate . Installer
-type Installer interface {
-	Install(string, string, string) error
-	Uninstall(string, string, string) error
-}
-
 // Reconcile handles events for the ByoHost that is registered by this agent process
 func (r *HostReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
 	logger := ctrl.LoggerFrom(ctx)
@@ -109,7 +104,9 @@ func (r *HostReconciler) reconcileNormal(ctx context.Context, byoHost *infrastru
 			return ctrl.Result{}, err
 		}
 
-		if r.K8sInstaller != nil {
+		if r.SkipInstallation {
+			logger.Info("Skipping installation of k8s components")
+		} else {
 			err = r.installK8sComponents(ctx, byoHost)
 			if err != nil {
 				logger.Error(err, "error in installing k8s components")
@@ -197,11 +194,17 @@ func (r *HostReconciler) hostCleanUp(ctx context.Context, byoHost *infrastructur
 		return err
 	}
 
-	if r.K8sInstaller != nil {
+	if r.SkipInstallation {
+		logger.Info("Skipping uninstallation of k8s components")
+	} else {
 		bundleRegistry := byoHost.GetAnnotations()[infrastructurev1beta1.BundleLookupBaseRegistryAnnotation]
 		k8sVersion := byoHost.GetAnnotations()[infrastructurev1beta1.K8sVersionAnnotation]
 		byohBundleTag := byoHost.GetAnnotations()[infrastructurev1beta1.BundleLookupTagAnnotation]
-		err = r.K8sInstaller.Uninstall(bundleRegistry, k8sVersion, byohBundleTag)
+		bundleInstaller, err := installer.New(bundleRegistry, r.DownloadPath, logger)
+		if err != nil {
+			return err
+		}
+		err = bundleInstaller.Uninstall(k8sVersion, byohBundleTag)
 		if err != nil {
 			return err
 		}
@@ -286,7 +289,11 @@ 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)
+	bundleInstaller, err := installer.New(bundleRegistry, r.DownloadPath, logger)
+	if err != nil {
+		return err
+	}
+	err = bundleInstaller.Install(k8sVersion, byohBundleTag)
 	if err != nil {
 		return err
 	}

agent/reconciler/reconciler_suite_test.go

@@ -12,7 +12,6 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/cloudinit/cloudinitfakes"
 	"github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/reconciler"
-	"github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/reconciler/reconcilerfakes"
 	infrastructurev1beta1 "github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/apis/infrastructure/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -40,7 +39,6 @@ var (
 	fakeCommandRunner  *cloudinitfakes.FakeICmdRunner
 	fakeFileWriter     *cloudinitfakes.FakeIFileWriter
 	fakeTemplateParser *cloudinitfakes.FakeITemplateParser
-	fakeInstaller      *reconcilerfakes.FakeInstaller
 )
 
 var _ = BeforeSuite(func() {