Fix installer Ubuntu_20.04.3_x86-64 bug (#243)

* Fix installer Ubuntu_20.04.3_x86-64 bug

Problem
When running cli with os flag set to "Ubuntu_20.04.3_x86-64", installer returns
"os k8s not supported". Installer is expected to support Ubuntu_20.04.*_x86-64
and treat them as Ubuntu_20.04.1_x86-64.

Rootcause
Installer runs a check whether there is at least one k8s for the os.
It does so by using registry ListK8s. However the latter expects that
its argument is an osBundle (Ubuntu_20.04.1_x86-64) as returned by
registry ListOS. There is no os bundle for "Ubuntu_20.04.3_x86-64".

Fix
registry ListK8s needs to support 2 use-cases. First, when called
with an actual OS to be mutated. Second, when called with list-supported
which may be on a non supported OS. Extend registry ListK8s to use the os
regex in addition to the os bundle. Extend registry to return the os bundle
in addition to installer.

agent/installer/installer.go

@@ -156,16 +156,18 @@ func (i *installer) Uninstall(k8sVer, tag string) error {
 func (i *installer) getAlgoInstallerWithBundle(k8sVer, tag string) (osk8sInstaller, error) {
 	// This OS supports at least 1 k8s version. See New.
 
-	algoInst := i.algoRegistry.GetInstaller(i.detectedOs, k8sVer)
+	algoInst, osBundle := i.algoRegistry.GetInstaller(i.detectedOs, k8sVer)
 	if algoInst == nil {
 		return nil, ErrOsK8sNotSupported
 	}
+	i.logger.Info("Current OS will be handled as", "OS", osBundle)
+
 	// copy installer from registry and set BundlePath including tag
 	// empty means preview mode
 	algoInstCopy := *algoInst.(*algo.BaseK8sInstaller)
 	algoInstCopy.BundlePath = i.bundleDownloader.getBundlePathDirOrPreview(k8sVer, tag)
 
-	bdErr := i.bundleDownloader.DownloadOrPreview(i.detectedOs, k8sVer, tag)
+	bdErr := i.bundleDownloader.DownloadOrPreview(osBundle, k8sVer, tag)
 	if bdErr != nil {
 		return nil, bdErr
 	}
@@ -198,7 +200,7 @@ func getSupportedRegistryDescription() registry {
 func PreviewChanges(os, k8sVer string) (install, uninstall string, err error) {
 	stepPreviewer := stringPrinter{msgFmt: "# %s"}
 	reg := getSupportedRegistry(&bundleDownloader{}, &stepPreviewer)
-	installer := reg.GetInstaller(os, k8sVer)
+	installer, _ := reg.GetInstaller(os, k8sVer)
 
 	if installer == nil {
 		err = ErrOsK8sNotSupported

agent/installer/installer_test.go

@@ -15,7 +15,7 @@ var _ = Describe("Byohost Installer Tests", func() {
 
 	Context("When installer is created for unsupported OS", func() {
 		It("Should return error", func() {
-			_, err := New("repo", "downloadPath", logr.Discard())
+			_, err := newUnchecked("Ubuntu_99.04.3_x86-64", "", "", logr.Discard(), nil)
 			Expect(err).Should((HaveOccurred()))
 		})
 	})
@@ -75,14 +75,19 @@ var _ = Describe("Byohost Installer Tests", func() {
 			Expect(osList).ShouldNot(BeEmpty())
 		})
 	})
-	Context("When ListSupportedK8s is called for all supported OSes", func() {
+	Context("When ListSupportedK8s is called for all supported bundle OSes", func() {
 		It("Should return non-empty result", func() {
 			_, osList := ListSupportedOS()
-			for _, os := range osList {
-				Expect(ListSupportedK8s(os)).ShouldNot(BeEmpty())
+			for _, osBundle := range osList {
+				Expect(ListSupportedK8s(osBundle)).ShouldNot(BeEmpty())
 			}
 		})
 	})
+	Context("When ListSupportedK8s is called for supported host OS", func() {
+		It("Should return non-empty result", func() {
+			Expect(ListSupportedK8s("Ubuntu_20.04.3_x86-64")).ShouldNot(BeEmpty())
+		})
+	})
 	Context("When PreviewChanges is called for all supported os and k8s", func() {
 		It("Should not return error", func() {
 			_, osList := ListSupportedOS()
@@ -116,6 +121,12 @@ var _ = Describe("Byohost Installer Tests", func() {
 			Expect(err).Should(Equal(ErrOsK8sNotSupported))
 		})
 	})
+	Context("When installer is created", func() {
+		It("Should be possible to do so using host os or bundle os ", func() {
+			Expect(func() { NewPreviewInstaller("Ubuntu_20.04.1_x86-64", nil) }).NotTo(Panic())
+			Expect(func() { NewPreviewInstaller("Ubuntu_20.04.3_x86-64", nil) }).NotTo(Panic())
+		})
+	})
 })
 
 func NewPreviewInstaller(os string, ob algo.OutputBuilder) *installer {

agent/installer/registry.go

@@ -57,16 +57,30 @@ func (r *registry) ListOS() (osFilter, osBundle []string) {
 	return
 }
 
-func (r *registry) ListK8s(osBundle string) []string {
-	result := make([]string, 0, len(r.osk8sInstallerMap[osBundle]))
-	for k8s := range r.osk8sInstallerMap[osBundle] {
+func (r *registry) ListK8s(osBundleHost string) []string {
+	var result []string
+
+	// os bundle
+	if k8sMap, ok := r.osk8sInstallerMap[osBundleHost]; ok {
+		for k8s := range k8sMap {
+			result = append(result, k8s)
+		}
+
+		return result
+	}
+
+	// os host
+	for k8s := range r.osk8sInstallerMap[r.resolveOsToOsBundle(osBundleHost)] {
 		result = append(result, k8s)
 	}
+
 	return result
 }
 
-func (r *registry) GetInstaller(osHost, k8sVer string) osk8sInstaller {
-	return r.osk8sInstallerMap[r.resolveOsToOsBundle(osHost)][k8sVer]
+func (r *registry) GetInstaller(osHost, k8sVer string) (osk8si osk8sInstaller, osBundle string) {
+	osBundle = r.resolveOsToOsBundle(osHost)
+	osk8si = r.osk8sInstallerMap[osBundle][k8sVer]
+	return
 }
 
 func (r *registry) resolveOsToOsBundle(os string) string {

agent/installer/registry_test.go

@@ -41,9 +41,17 @@ var _ = Describe("Byohost Installer Tests", func() {
 			r.AddOsFilter("ubuntu.*", "ubuntu")
 			r.AddOsFilter("rhel.*", "rhel")
 
-			Expect(r.GetInstaller("ubuntu-1", "1.22")).To(Equal(dummy122))
-			Expect(r.GetInstaller("ubuntu-2", "1.23")).To(Equal(dummy123))
-			Expect(r.GetInstaller("rhel-1", "1.24")).To(Equal(dummy124))
+			inst, osBundle := r.GetInstaller("ubuntu-1", "1.22")
+			Expect(inst).To(Equal(dummy122))
+			Expect(osBundle).To(Equal("ubuntu"))
+
+			inst, osBundle = r.GetInstaller("ubuntu-2", "1.23")
+			Expect(inst).To(Equal(dummy123))
+			Expect(osBundle).To(Equal("ubuntu"))
+
+			inst, osBundle = r.GetInstaller("rhel-1", "1.24")
+			Expect(inst).To(Equal(dummy124))
+			Expect(osBundle).To(Equal("rhel"))
 
 			osFilters, osBundles := r.ListOS()
 			Expect(osFilters).To(ContainElements("rhel.*", "ubuntu.*"))
@@ -65,6 +73,29 @@ var _ = Describe("Byohost Installer Tests", func() {
 			Expect(osBundles).To(HaveLen(2))
 
 		})
+		It("Should decouple host os from bundle os", func() {
+			// Bundle OS does not match filter OS
+			r.AddBundleInstaller("UBUNTU", "1.22", dummy122)
+			r.AddOsFilter("ubuntu.*", "UBUNTU")
+
+			inst, osBundle := r.GetInstaller("ubuntu-1", "1.22")
+			Expect(inst).To(Equal(dummy122))
+			Expect(osBundle).To(Equal("UBUNTU"))
+
+			// ListOS should return only bundle OS
+			_, osBundles := r.ListOS()
+			Expect(osBundles).To(ContainElements("UBUNTU"))
+			Expect(osBundles).To(HaveLen(1))
+
+			// ListK8s should work with both
+			osBundleResult := r.ListK8s("UBUNTU")
+			Expect(osBundleResult).To(ContainElements("1.22"))
+			Expect(osBundleResult).To(HaveLen(1))
+
+			osHostResult := r.ListK8s("ubuntu-20-04")
+			Expect(osHostResult).To(ContainElements("1.22"))
+			Expect(osHostResult).To(HaveLen(1))
+		})
 		It("Should panic on duplicate installers", func() {
 			/*
 			 * Add is expected to be called with literals only.