-
Notifications
You must be signed in to change notification settings - Fork 37
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
fix: do not preserve extensions on Talos agent mode #799
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest to add some tests. We have unit tests for the extensions discovery, it should be a good place to add it.
internal/backend/runtime/omni/controllers/omni/internal/talos/schematic.go
Show resolved
Hide resolved
If Talos is in agent mode, we should avoid preserving the list of extensions for the actual Talos installation, as the `metal-agent` extension breaks the Talos installation. Signed-off-by: Utku Ozdemir <[email protected]>
68a68e5
to
1d8c754
Compare
id := "test-" + tt.name | ||
|
||
machine := omni.NewMachine(resources.DefaultNamespace, id) | ||
spec := machine.TypedSpec().Value | ||
|
||
spec.Connected = true | ||
spec.ManagementAddress = suite.socketConnectionString | ||
|
||
suite.Require().NoError(suite.state.Create(suite.ctx, machine)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this here to create a different machine ID each time - otherwise MachineStatus
preserves the initial schematic, and my new test was giving different results when it was run in isolation vs when it was run together with other tests.
done |
/m |
If Talos is in agent mode, we should avoid preserving the list of extensions for the actual Talos installation, as the
metal-agent
extension breaks the Talos installation.Related to/workaround for #797