Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Installer OS mapping #207

Merged
merged 3 commits into from
Nov 5, 2021

Conversation

ratanasovvmw
Copy link
Contributor

@ratanasovvmw ratanasovvmw commented Nov 3, 2021

A BYOH bundle and installer Algo are developed and tested using a specific OS version (including patch version).
This is why the patch version is included in the BYOH bundle name. An old BYOH Bundle may work with newer OS patches and versions.

This PR make the installer smart. It can map any OS version to any BYOH Bundle and installer Algo.

Registry contains

  1. Entries associating BYOH Bundle in the Repository with Installer in Host Agent
  2. Entries that match an OS to a BYOH Bundle OS in the Repository

cli --list-supported

OS                      K8S Version     BYOH Bundle Name
---                     -----------     ----------------
Ubuntu_20.04.*_x86-64    v1.22.1        byoh-bundle-ubuntu_20.04.1_x86-64_k8s_v1.22.1

Testing done
On a Ubuntu 18.04
./cli --list-supported
./cli --preview-os-changes --os Ubuntu_20.04.*_x86-64 --k8s v1.22.1
./cli --install --os Ubuntu_20.04.1_x86-64 --k8s v1.22.1 --bundle-repo
./cli --uninstall --os Ubuntu_20.04.1_x86-64 --k8s v1.22.1 --bundle-repo

A BYOH bundle and installer Algo are developed and tested using a specific OS version (including patch version).
This is why the patch version is included in the BYOH bundle name. An old BYOH Bundle may work with newer OS patches and versions.

This PR make the installer smart. It can map any OS version to any BYOH Bundle and installer Algo.

Example:
A BYOH Bundle and installer Algo are created for Ubuntu_20.04.1_x86-64. They will also be used on all Ubuntu 20.04 patches.

OS                      K8S Version     BYOH Bundle Name
---                     -----------     ----------------
Ubuntu_20.04.1_x86-64    v1.22.1        byoh-bundle-ubuntu_20.04.1_x86-64_k8s_v1.22.1
Ubuntu_20.04.*_x86-64    v1.22.1        byoh-bundle-ubuntu_20.04.1_x86-64_k8s_v1.22.1
agent/installer/installer.go Outdated Show resolved Hide resolved
agent/installer/registry.go Outdated Show resolved Hide resolved
1. Entries associating BYOH Bundle in the Repository with Installer in Host Agent
2. Entries that match an OS to a BYOH Bundle OS in the Repository

cli --list-supported
OS                      K8S Version     BYOH Bundle Name
---                     -----------     ----------------
Ubuntu_20.04.*_x86-64    v1.22.1        byoh-bundle-ubuntu_20.04.1_x86-64_k8s_v1.22.1
@ratanasovvmw ratanasovvmw marked this pull request as ready for review November 4, 2021 14:20
OutputBuilder: ob}
reg.Add(t.os, t.k8s, a)
// Match concrete os version to repository os version
reg.AddOsFilter("Ubuntu_20.04.*_x86-64", linuxDistro)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question - do we need to hard code this? Can this be injected/overridden via a config file? This could help in our integration testing too.

Copy link
Contributor Author

@ratanasovvmw ratanasovvmw Nov 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be made configurable when needed.

Technically speaking

  • only the "Ubuntu_20.04.*_x86-64" can be configurable
  • the "Ubuntu_20.04.1_x86-64" cannot be configurable because it is bound to a struct which may be different for OSes and versions. It is the core of the installer

Workflow speaking,

  • yixingjia implies that this lib is the source of truth and needs to be queried
  • if it is configurable whose responsibility would be to do that?

Testing speaking,

  • my experience so far is that we need to override the detection of the current OS. E.g. if lib has ubuntu2004 and I'm running on ubuntu1804, I can override and treat it as ubuntu2004
  • I suggest to make configurable when we have a concrete use-case

Copy link
Contributor Author

@ratanasovvmw ratanasovvmw Nov 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a meeting with Dharmjit and Yixing where config files were also mentioned but regarding that downstream version for k8s is 1.2.1+vmware.1. Some component needs to be able to understand upstream and downstream k8s versions like (1.2.1+vmware.1). Such component can be a config file (which is similar to fork)
Agreed we need to look at this holistically post FC.

)

var (
listSupportedFlag = flag.Bool("list-supported", false, "List all supported OS and Kubernetes versions")
listBundlesFlag = flag.Bool("list-bundles", false, "List the BYOH Bundle names for all supported OS and Kubernetes versions")
listSupportedFlag = flag.Bool("list-supported", false, "List all supported OS, Kubernetes versions and BYOH Bundle names")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question, maybe I'm wrong :)
Should we add the list support bundles function in the management cluster side?

If I'm an end user, after I install the byoh provider , then I will expect to check what bundles do we have in the repo on the management cluster side instead of login to the remote byoh host.

Then I can decide which version of kubernest/ bundle I would like to use to create the cluster on byoh host.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. This is just a CLI of the installer lib that is used by the host agent. So, the CP can retrieve the same info by the lib directly. The exact workflow for that is outside installer lib charter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@dharmjit dharmjit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good for now, But as @ratanasovvmw mentioned we need to look at it again from the UX perspective.

@ratanasovvmw ratanasovvmw merged commit 7e93f86 into vmware-tanzu:main Nov 5, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants