-
-
Notifications
You must be signed in to change notification settings - Fork 763
feat: implement KubeSpan multi-document configuration #12504
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
base: main
Are you sure you want to change the base?
feat: implement KubeSpan multi-document configuration #12504
Conversation
| } | ||
|
|
||
| // machineConfigWrapper wraps MachineConfig to override Network().KubeSpan() with multi-doc aggregation. | ||
| type machineConfigWrapper struct { |
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.
this approach doesn't scale well.
we have this pattern:
talos/pkg/machinery/config/container/container.go
Lines 326 to 341 in 1549521
| // NetworkTimeSyncConfig implements config.Config interface. | |
| func (container *Container) NetworkTimeSyncConfig() config.NetworkTimeSyncConfig { | |
| // first check if we have a dedicated document | |
| matching := findMatchingDocs[config.NetworkTimeSyncConfig](container.documents) | |
| if len(matching) > 0 { | |
| return matching[0] | |
| } | |
| // fallback to v1alpha1 | |
| if container.v1alpha1Config != nil { | |
| return container.v1alpha1Config.NetworkTimeSyncConfig() | |
| } | |
| return nil | |
| } | |
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.
Removed the wrapper in wrapper pattern, and followed existing one. 👍
| // KubeSpanFiltersConfig configures KubeSpan endpoint filters. | ||
| type KubeSpanFiltersConfig struct { | ||
| // description: | | ||
| // Filter node addresses which will be advertised as KubeSpan endpoints for peer-to-peer Wireguard connections. |
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.
this needs more configuration/examples
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.
Updated 👍
| return *s.ConfigMTU | ||
| } | ||
|
|
||
| return 1420 |
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 think we had a constant for it.
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.
yes, found it KubeSpanLinkMTU
|
|
||
| // Endpoints implements config.KubeSpanFilters interface. | ||
| func (f *KubeSpanFiltersConfig) Endpoints() []string { | ||
| if f == nil { |
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.
this can be removed (nil slice is an empty slice).
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.
removed 👍
| } | ||
|
|
||
| // NetworkConfig represents the machine's networking config values. | ||
| type NetworkConfig struct { |
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.
we can now deprecate the whole .machine.network tree, so that it disappears from the docs (it's now empty as everything inside it is deprecated).
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.
Updated with docgen: nodoc
smira
left a comment
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.
talosctl cluster createhas--with-kubepsanoption which should use new multi-doc for Talos >= 1.13 (need new config version contract flag)- KubeSpanConfigController should have a new testcase for enabling KubeSpan via new multi-doc
Migrate KubeSpan configuration to support multi-document format. Add version-aware support for talosctl cluster create and gen config. Uses multi-doc format for Talos 1.13+, legacy format for 1.12 and earlier. Signed-off-by: Pranav Patil <[email protected]>
045777a to
bc95639
Compare
|
| genOptions = slices.Concat(genOptions, | ||
| []generate.Option{generate.WithNetworkOptions( | ||
| v1alpha1.WithKubeSpan(), | ||
| )}, |
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'm aware that we're migrating .machine.network to multi-document format, but I've kept legacy v1alpha1 config generation for Talos < 1.13 as previous versions can't parse new KubeSpanConfigV1Alpha1
Resolves #12437
Pull Request
What? (description)
KubeSpanConfigV1Alpha1document type with registry supportWhy? (reasoning)
Making/applying config for kubespan easier,
instead of the current scenario which requires:
We can have independent config for Kubespan like:
Few more Points for my own sanity:
deprecate and hide fully .machine.network., but this still supports the previous config way, this just extends and gives priority to the multi-document config way for Kubespan.pkg/machinery/config/types/network/kubespan.gowhich reads yaml and createsKubeSpanConfigV1Alpha1(struct) with all values and implements all interfaces.pkg/machinery/config/container/container.goto use a wrapper-in-wrapper pattern instead of just returningcontainer.v1alpha1Config.Machine()helps to intercept controller call oncontainer.Machine().Network().KubeSpan()and use the desired (new multi-doc config) fromcontainer.documentsmake generateCreated json and copy stuff.and following above pattern we have such structs, but https://github.com/siderolabs/talos/blob/main/api/resource/definitions/kubespan/kubespan.proto has
also the config for kubespan controller has
https://github.com/siderolabs/talos/blob/main/internal/app/machined/pkg/controllers/kubespan/config.go#L49
res.TypedSpec().ForceRouting = c.Machine().Network().KubeSpan().ForceRouting()forceRouting instead of allowDownPeerBypass,which leads to https://github.com/siderolabs/talos/compare/main...pranav767:talos:feat/kubespan-multi-document-config?expand=1#diff-2f6f6aab6dbbcc38f191c5f752866e9d8d3dc787f0044c4eb9d7107ea7a1719dR170
Just feels a bit of inconsistency with the use of one parameter with 2 different names.
Acceptance
Please use the following checklist:
make conformance)make fmt)make lint)make docs)make unit-tests)