Implement HA RKE2 architecture #124
Conversation
atanasdinov
commented
Jan 22, 2024
•
atanasdinov
commented
- Splits RKE2 configuration based on number of nodes
- Introduces a MetalLB + Endpoint Copier Operator manifest in order to create a VIP address for the cluster
- Establishes sane default values for servers and agents in both single and multinode setup
- Generating a cluster token to be done in a follow up
| @@ -0,0 +1,76 @@ | |||
| --- | |||
There was a problem hiding this comment.
I'd eventually look into completely replacing most of this with the addition of manifests and charts support.
atanasdinov
force-pushed
the
rke2-ha
branch
2 times, most recently
from
97fc5c2
to
f956eb2
Compare
Signed-off-by: Atanas Dinov <[email protected]>
Signed-off-by: Atanas Dinov <[email protected]>
Signed-off-by: Atanas Dinov <[email protected]>
Signed-off-by: Atanas Dinov <[email protected]>
Signed-off-by: Atanas Dinov <[email protected]>
Signed-off-by: Atanas Dinov <[email protected]>
Signed-off-by: Atanas Dinov <[email protected]>
Signed-off-by: Atanas Dinov <[email protected]>
Signed-off-by: Atanas Dinov <[email protected]>
Signed-off-by: Atanas Dinov <[email protected]>
| @@ -48,15 +48,9 @@ func validateNodes(k8s *image.Kubernetes) []FailedValidation { | |||
| }) | |||
| } | |||
|
|
|||
| if k8s.Network.APIHost == "" { | |||
| failures = append(failures, FailedValidation{ | |||
| UserMessage: "The 'apiHost' field is required in the 'network' section when defining entries under 'nodes'.", | |||
There was a problem hiding this comment.
We decided on having this field as optional.
Signed-off-by: Atanas Dinov <[email protected]>
Signed-off-by: Atanas Dinov <[email protected]>
Signed-off-by: Atanas Dinov <[email protected]>
Signed-off-by: Atanas Dinov <[email protected]>
There was a problem hiding this comment.
Nothing blocking this from landing. I do think we should discuss the definition spelling, but that can come next week. I still haven't seen this actually working, but it a.) doesn't actively break anything and b.) looks sound from a coding perspective, so I'm cool with landing this and debugging my issue next week.
| First bool `yaml:"firstNode"` | ||
| Hostname string `yaml:"hostname"` | ||
| Type string `yaml:"type"` | ||
| Initialiser bool `yaml:"initialiser"` |
There was a problem hiding this comment.
It's not worth blocking the PR for this, but we should discuss the spelling of "initializer" in the image definition (I'm actually being serious this time, not just giving you crap). I think it's potentially confusing that embeddedArtifactRegistry uses the Americanized spelling while this uses whatever you call the other spelling version (I know, I know, your answer is going to be "correct"). In the code I don't care if we mix and match, but for a public API I think it looks bad if it's not consistent. We can talk about this in the definition review next week.
pkg/kubernetes/cluster.go
Outdated
| type Cluster struct { | ||
| // Initialiser is the hostname of the initialiser node. | ||
| // Defaults to the first configured server if not explicitly selected. | ||
| Initialiser string |
There was a problem hiding this comment.
Nit: In the definition, Initialiser is a boolean, but here it's the hostname. There shouldn't be any technical confusion since they are different types, though it might be a bit more clear to new devs if this was InitialiserHostname (esp since it matches the pattern of InitialiserConfig below).
pkg/kubernetes/cluster.go
Outdated
| return | ||
| } | ||
|
|
||
| auditMessage := fmt.Sprintf("Kubernetes CNI not explicitly set, defaulting to: %s", cniDefaultValue) |
There was a problem hiding this comment.
Nit: The user-facing messages have been closer to full sentences in many places, so we may want to consider:
"The Kubernetes CNI is not explicitly set, defaulting to '%s'."
Signed-off-by: Atanas Dinov <[email protected]>
