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

Add ByoHost CR with the Host platform information #470

Closed
dharmjit opened this issue Apr 4, 2022 · 5 comments · Fixed by #516
Closed

Add ByoHost CR with the Host platform information #470

dharmjit opened this issue Apr 4, 2022 · 5 comments · Fixed by #516
Assignees
Milestone

Comments

@dharmjit
Copy link
Contributor

dharmjit commented Apr 4, 2022

After #469, we need to add the host platform details while registering ByoHost CR in the Host-Agent.

@shubham14bajpai
Copy link
Contributor

@dharmjit @anusha94 For this do we need to write an interface for different OS (linux, windows) to be future ready or
we can just go ahead with linux for now and make it logic independent later as we start supporting more OS?
Also should the agent fail if it detects an unsupported OS/Arch?

@sachinkumarsingh092
Copy link
Contributor

I think this is a good idea to be future-proof this way, but everything now is based on the assumption of the OS being an Ubuntu Linux. The current os-detection for the agent (which is a good reference for this task) assumes this only.

What we can do for this task, is to follow the trend and do this for Ubuntu Linux for now. Later on, we can make a task/effort to have these interfaces for every logic for os detection. This way it would be a uniform and consistent change throughout the codebase.

Also should the agent fail if it detects an unsupported OS/Arch?

We can have similar logic or even modularise this logic out of the installer to be reused in both the installer and during registration.

What do you folks think?

@dharmjit
Copy link
Contributor Author

dharmjit commented Apr 20, 2022

Hi @shubham14bajpai @sachin, I would suggest that we should at least target Linux-based Operating systems. I tried to look at how the kubelet is fetching this information and it uses goruntime GOOS, GOARCH fields for OS/Arch and for the OSImage we can use /etc/os-release(I think this is a standardized file across Linux system but you could double-check on this.)
To achieve ^^, I guess we don't need to write interfaces as of now.

@shubham14bajpai
Copy link
Contributor

Hi @shubham14bajpai @sachin, I would suggest that we should at least target Linux-based Operating systems. I tried to look at how the kubelet is fetching this information and it uses goruntime GOOS, GOARCH fields for OS/Arch and for the OSImage we can use /etc/os-release(I think this is a standardized file across Linux system but you could double-check on this.)

Yes I was looking at the same as the runtime package provides us with the details very easily. I guess we don't need the interfaces right now as there is no talk on windows support. Will proceed with the above approach then.

@davidspek
Copy link

I’m not sure how relevant this will be for this infrastructure provider, but it might also be nice to implement https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20210310-opt-in-autoscaling-from-zero.md since the given node information can also be useful elsewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment