feat: add extended hardware information to Server and ServerClass CRDs #735

lion7 · 2022-02-01T13:21:10Z

No description provided.

netlify · 2022-02-01T13:21:13Z

👷 Deploy request for wonderful-swartz-a1308c pending review.
Visit the deploys page to approve it

🔨 Explore the source changes: 3571b80

smira · 2022-02-02T12:53:35Z

app/sidero-controller-manager/api/v1alpha1/server_types.go

-	Cordoned          bool                    `json:"cordoned,omitempty"`
-	PXEBootAlways     bool                    `json:"pxeBootAlways,omitempty"`
+	EnvironmentRef *corev1.ObjectReference `json:"environmentRef,omitempty"`
+	Hardware       *HardwareInformation    `json:"hardware,omitempty"`


this is a breaking change for the CRDs, as the fields were removed. We could do also migration and new CRD version, but that might need more changes. I wonder if we could still keep things same way in the CRDs, even if we restructure the fields in Go structs.

Yeah, that is also what I was struggling with. I agree not breaking things is better, but I honestly don't know a good way.
A partial migration should be possible I think, although it will probably always lead to incomplete information (for example servers with multiple CPU's will only have 1 listed). Also we would only have a few fields of the new set available.
I'm not sure what is worse: not having information or having incomplete information...

👍 for a new CRD version and we do CRD migrations, it'll be less visible changes for a user

I agree we should probably do new API version and migration, but this means we should also fix the webhooks for this controller (it's right now broken/missing) and go all the way with proper migration. And if we do that, should we do more changes as well?

smira · 2022-02-03T14:08:58Z

@lion7 we really like the proposed changes here, but as we would need to do CRD migration, we would like to delay that for the next release of Sidero (0.6.0). We need to fix the webhooks first in the controllers and implement proper migration

lion7 · 2022-02-03T14:38:03Z

I totally agree. Is there a timeline on the next release? Like in the order of days / weeks / months?

Anyway, I'll see if I can improve this PR by implementing the proposed changes.

lion7 · 2022-02-03T20:28:00Z

I moved my CRD changes to a new version and implemented the manual conversions that were needed. This should not break things I hope.

I am not entirely sure about the whole conversion part though. The conversions for the caps-controller-manager also have these func (src *MetalCluster) ConvertTo(dstRaw conversion.Hub) error {...} and func (dst *MetalCluster) ConvertFrom(srcRaw conversion.Hub) error {...} functions which I didn't yet implement for the sidero-controller-manager.
Nothing seems to break when building, is this something that's used only at runtime inside a K8s cluster? (haven't tested that yet)

lion7 · 2022-03-26T16:09:40Z

I also added network interfaces to the collected information, so we have the complete set of data from servers.

@smira there are some (un)related changes in this PR that I'd like to separate in other PR's so the diff will be smaller.
For example, I had to update all imports due to the new v1alpha2 CRD's and found a lot of places with inconsistent naming for imports. I have moved most of these changes into #793.

I also want to move the Kustomize config changes to a different PR, there are quite a few of them in order to get the whole webhook stuff working. Still working on that one.

lion7 · 2022-03-28T09:59:04Z

Finally got the conversion webhooks working 🥳
I duplicated these changes in #794, so they can be merged independently.
After #793, #794 and #795 are merged I'll update this PR so the diff will only cover the actual changes of adding the extended hardware information as the title suggests.

Display the extendend hardware information that was added via siderolabs/sidero#735 in the Theila UI Signed-off-by: Gerard de Leeuw <gdeleeuw@leeuwit.nl>

This change adds detailed hardware information to the Server CRD. Hardware info is extracted by the agent from SMBIOS. The ServerClass CRD is also updated so more precise qualifiers can be used. Signed-off-by: Gerard de Leeuw <gdeleeuw@leeuwit.nl>

Display the extendend hardware information that was added via siderolabs/sidero#735 in the Theila UI Signed-off-by: Gerard de Leeuw <gdeleeuw@leeuwit.nl>

Display the extendend hardware information that was added via siderolabs/sidero#735 in the Theila UI Co-authored-by: Gerard de Leeuw <gdeleeuw@leeuwit.nl> Signed-off-by: Artem Chernyshev <artem.chernyshev@talos-systems.com>

This is basically subset of PR siderolabs#735 with only CRD changes without actual code changes to use new CRDs. As storage version is v1alpha2, every access goes twice via conversion webhooks (for better test coverage). Other parts of siderolabs#735 will be incorporated in a follow-up PR, I decided to split things up for easier review. Example: ```bash $ kubectl get servers.v1alpha1.metal.sidero.dev 49fd7c2d-1ba4-4157-8cc0-3f7212f119f0 -o yaml ... cpu: manufacturer: QEMU version: pc-q35-6.0 hostname: pxe-3 managementApi: endpoint: 172.25.0.1:39565 system: family: Unknown manufacturer: QEMU productName: Standard PC (Q35 + ICH9, 2009) serialNumber: Unknown skuNumber: Unknown version: pc-q35-6.0 $ kubectl get servers.v1alpha2.metal.sidero.dev 49fd7c2d-1ba4-4157-8cc0-3f7212f119f0 -o yaml ... hardware: compute: processors: - manufacturer: QEMU productName: pc-q35-6.0 system: family: Unknown manufacturer: QEMU productName: Standard PC (Q35 + ICH9, 2009) serialNumber: Unknown skuNumber: Unknown version: pc-q35-6.0 ``` We can make more changes to v1alpha2 resources in follow-up PRs. Co-authored-by: Gerard de Leeuw <gdeleeuw@leeuwit.nl> Signed-off-by: Gerard de Leeuw <gdeleeuw@leeuwit.nl> Signed-off-by: Andrey Smirnov <andrey.smirnov@talos-systems.com>

This is final part of siderolabs#735, previous part in siderolabs#823. This imports all remaining changes with some fixups minus the webhook changes. This change adds detailed hardware information to the Server CRD. Hardware info is extracted by the agent from SMBIOS. The ServerClass CRD is also updated so more precise qualifiers can be used. Co-authored-by: Gerard de Leeuw <gdeleeuw@leeuwit.nl> Signed-off-by: Gerard de Leeuw <gdeleeuw@leeuwit.nl> Signed-off-by: Andrey Smirnov <andrey.smirnov@talos-systems.com>

smira · 2022-04-15T18:53:41Z

@lion7 I want to say once again thank you for your awesome job on this one. Even in the test environment the reporting of hardware information is much improved!

Sorry that it took too long to merge this, but we got these changes merged with some fixups in #823 and #824

I haven't made any changes to the actual CRDs, but rather some small fixups here and there, so I believe this PR might be closed unless I missed something on the way :)

lion7 · 2022-04-16T08:38:45Z

@smira great to hear that this feature was merged! No problem on the delay, it was a huge change (and PR) after all 😅.
It all looks good to me, I'll do a local build and test it out on my setup. Closing this PR 🥳.

lion7 force-pushed the master branch from 1ab7fc9 to ffd3c2d Compare February 1, 2022 13:26

smira reviewed Feb 2, 2022

View reviewed changes

lion7 force-pushed the master branch 3 times, most recently from f28d80c to e5d71a7 Compare February 3, 2022 20:24

lion7 force-pushed the master branch 3 times, most recently from 1a0dc09 to b0cd6ea Compare February 7, 2022 09:10

lion7 force-pushed the master branch 3 times, most recently from 1b80b9a to 3571b80 Compare February 17, 2022 19:24

lion7 force-pushed the master branch 5 times, most recently from bf99ecc to e72fead Compare March 26, 2022 15:57

lion7 force-pushed the master branch 7 times, most recently from ad036ed to 1136eb0 Compare March 28, 2022 09:31

lion7 mentioned this pull request Mar 28, 2022

feat: fix conversion webhooks #794

Closed

lion7 added a commit to lion7/theila that referenced this pull request Mar 30, 2022

feat: show the extended hardware info

09f604b

Display the extendend hardware information that was added via siderolabs/sidero#735 in the Theila UI Signed-off-by: Gerard de Leeuw <gdeleeuw@leeuwit.nl>

lion7 mentioned this pull request Mar 30, 2022

feat: show the extended hardware info siderolabs/theila#128

Merged

lion7 added a commit to lion7/theila that referenced this pull request Mar 30, 2022

feat: show the extended hardware info

d1c4ae3

Display the extendend hardware information that was added via siderolabs/sidero#735 in the Theila UI Signed-off-by: Gerard de Leeuw <gdeleeuw@leeuwit.nl>

lion7 force-pushed the master branch 2 times, most recently from 9208c33 to 68482af Compare March 31, 2022 17:44

lion7 force-pushed the master branch from 68482af to 4367246 Compare March 31, 2022 18:45

lion7 added a commit to lion7/theila that referenced this pull request Mar 31, 2022

feat: show the extended hardware info

5f7c114

Display the extendend hardware information that was added via siderolabs/sidero#735 in the Theila UI Signed-off-by: Gerard de Leeuw <gdeleeuw@leeuwit.nl>

lion7 mentioned this pull request Apr 13, 2022

feat: enable webhooks in sidero-controller-manager #812

Merged

smira mentioned this pull request Apr 14, 2022

feat: restructure HW information in Server resources (v1alpha2) #823

Merged

smira mentioned this pull request Apr 14, 2022

feat: add extended hardware information to Server and ServerClass CRDs #824

Merged

lion7 closed this Apr 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add extended hardware information to Server and ServerClass CRDs #735

feat: add extended hardware information to Server and ServerClass CRDs #735

lion7 commented Feb 1, 2022

netlify bot commented Feb 1, 2022 •

edited

Loading

smira Feb 2, 2022

lion7 Feb 2, 2022

frezbo Feb 2, 2022

smira Feb 2, 2022

smira commented Feb 3, 2022

lion7 commented Feb 3, 2022 •

edited

Loading

lion7 commented Feb 3, 2022 •

edited

Loading

lion7 commented Mar 26, 2022

lion7 commented Mar 28, 2022

smira commented Apr 15, 2022

lion7 commented Apr 16, 2022

feat: add extended hardware information to Server and ServerClass CRDs #735

feat: add extended hardware information to Server and ServerClass CRDs #735

Conversation

lion7 commented Feb 1, 2022

netlify bot commented Feb 1, 2022 • edited Loading

smira Feb 2, 2022

Choose a reason for hiding this comment

lion7 Feb 2, 2022

Choose a reason for hiding this comment

frezbo Feb 2, 2022

Choose a reason for hiding this comment

smira Feb 2, 2022

Choose a reason for hiding this comment

smira commented Feb 3, 2022

lion7 commented Feb 3, 2022 • edited Loading

lion7 commented Feb 3, 2022 • edited Loading

lion7 commented Mar 26, 2022

lion7 commented Mar 28, 2022

smira commented Apr 15, 2022

lion7 commented Apr 16, 2022

netlify bot commented Feb 1, 2022 •

edited

Loading

lion7 commented Feb 3, 2022 •

edited

Loading

lion7 commented Feb 3, 2022 •

edited

Loading