From 29c5b0601bd907f6721d8afd9af3b0aa420affab Mon Sep 17 00:00:00 2001 From: ratanasovvmw <86951923+ratanasovvmw@users.noreply.github.com> Date: Wed, 17 Nov 2021 08:30:55 +0200 Subject: [PATCH] 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 | 8 ++++--- agent/installer/installer_test.go | 19 ++++++++++++---- agent/installer/registry.go | 24 +++++++++++++++----- agent/installer/registry_test.go | 37 ++++++++++++++++++++++++++++--- 4 files changed, 73 insertions(+), 15 deletions(-) diff --git a/agent/installer/installer.go b/agent/installer/installer.go index dabe55207..6313d190b 100644 --- a/agent/installer/installer.go +++ b/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 diff --git a/agent/installer/installer_test.go b/agent/installer/installer_test.go index 651bd014b..ae348266b 100644 --- a/agent/installer/installer_test.go +++ b/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 { diff --git a/agent/installer/registry.go b/agent/installer/registry.go index ecd799ae1..e09f13302 100644 --- a/agent/installer/registry.go +++ b/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 { diff --git a/agent/installer/registry_test.go b/agent/installer/registry_test.go index 7156c6e42..dfd5c70b9 100644 --- a/agent/installer/registry_test.go +++ b/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.