Integrate host-agent with the installer #216
Conversation
- introduce a new flag skip-installation for the host-agent. The site operators can use this flag if they wish to manage the k8s components themselves - refactor installer to update New, Install and Uninstall signatures Signed-off-by: Anusha Hegde <anushah@vmware.com>
| } | ||
|
|
||
| // Install installs the specified k8s version on the current OS | ||
| func (i *installer) Install(k8sVer, tag string) error { | ||
| func (i *installer) Install(bundleRepo, k8sVer, tag string) error { | ||
| i.bundleDownloader = bundleDownloader{repoAddr: bundleRepo, downloadPath: i.downloadPath, logger: i.logger} |
There was a problem hiding this comment.
it's bad design that this method modifies the installer object. Same for uninstall.
There was a problem hiding this comment.
Agreed. This needs to be refactored. I think the old assumption was that we would have the repoAddr when the host-agent was started. But this assumption was false. The repoAddr will be passed in at Install / Uninstall time, so there is no point saving it. We just took the path of least resistance to get the installer integrated.
There was a problem hiding this comment.
Created #221 to track this work
There was a problem hiding this comment.
I acknowledge the least resistance for FC. That was to New every time. A much smaller and safer change.
| if bundleRepo == "" { | ||
| return nil, fmt.Errorf("empty bundle repo") | ||
| } | ||
| func New(downloadPath string, logger logr.Logger) (*installer, error) { |
There was a problem hiding this comment.
downloadPath goes with bundleRepo. It's a cache of the bundle repo. It's odd that they are decoupled in different methods.
There was a problem hiding this comment.
I thought downloadPath is a location on disk to cache the repo. Why would it be coupled to the bundleRepo?
There was a problem hiding this comment.
Indeed "cache the repo" - yes. After this change there is no "the repo"
This change makes the cache shareable between repos. If they happen to have different bundles with the same k8s version, this will simply not work as they will clash and will have false hits. Installer assumes 1 repo, 1 local cache. This change is half-making another model.
In a model where there are different repos, each of them must have it's own cache. So no collisions are possible.
|
We could open a linked Issue in case there are some code improvements or code refactoring is required. |
Signed-off-by: Anusha Hegde anushah@vmware.com
What this PR does / why we need it:
operators can use this flag if they wish to manage the k8s components
themselves
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #204