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 machine names as etcd name #778

Merged
merged 1 commit into from
Oct 29, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import (
bootstrapv1 "github.com/k0sproject/k0smotron/api/bootstrap/v1beta1"
"github.com/k0sproject/k0smotron/internal/cloudinit"
kutil "github.com/k0sproject/k0smotron/internal/util"
"github.com/k0sproject/version"
)

type ControlPlaneController struct {
Expand All @@ -63,6 +64,8 @@ type ControlPlaneController struct {

const joinTokenFilePath = "/etc/k0s.token"

var minVersionForETCDName = version.MustParse("v1.31.1+k0s.0")

// +kubebuilder:rbac:groups=bootstrap.cluster.x-k8s.io,resources=k0scontrollerconfigs,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=bootstrap.cluster.x-k8s.io,resources=k0scontrollerconfigs/status,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters;clusters/status;machines;machines/status,verbs=get;list;watch;create;update;patch;delete
Expand Down Expand Up @@ -155,6 +158,7 @@ func (c *ControlPlaneController) Reconcile(ctx context.Context, req ctrl.Request
)

if config.Spec.K0s != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few thoughts:

  • Should we set the field even if user didn't provide us any custom k0s config?
  • At the end of this if statement, we call kutil.ReconcileDynamicConfig, but we probably need to remove the etcd node name from the DynamicConfig.
  • Should the most of the code of this statement be in the control plane controller instead of bootstrap controller?

Copy link
Contributor Author

@apedriza apedriza Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Should we set the field even if user didn't provide us any custom k0s config?

Agree. It is already used etcd node name as default value if kine is not set so I think is safe assume that. Added

  • At the end of this if statement, we call kutil.ReconcileDynamicConfig, but we probably need to remove the etcd node name from the DynamicConfig.

Im not sure if I follow your point. It is not removed spec.storage in kutil.ReconcileDynamicConfig from the DynamicConfig?

  • Should the most of the code of this statement be in the control plane controller instead of bootstrap controller?

If you mean to move most of the logic to the place where we create bootstrapv1.K0sControllerConfig Im ok with that. But I think that change should be done carefully, Im not sure exactly what it might imply rn. Maybe addressing that refactoring in another PR makes more sense, wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not sure if I follow your point. It is not removed spec.storage in kutil.ReconcileDynamicConfig from the DynamicConfig?

Sorry, missed it. Yeah, that's good then.

Maybe addressing that refactoring in another PR makes more sense, wdyt?

Sorry, I was not clear enough: of course, it's better to do it in a separate PR.

Copy link
Contributor Author

@apedriza apedriza Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ive created an issue for that refactor #798


nllbEnabled, found, err := unstructured.NestedBool(config.Spec.K0s.Object, "spec", "network", "nodeLocalLoadBalancing", "enabled")
if err != nil {
return ctrl.Result{}, fmt.Errorf("error getting nodeLocalLoadBalancing: %v", err)
Expand Down Expand Up @@ -190,17 +194,6 @@ func (c *ControlPlaneController) Reconcile(ctx context.Context, req ctrl.Request
}
}

k0sConfigBytes, err := config.Spec.K0s.MarshalJSON()
if err != nil {
return ctrl.Result{}, fmt.Errorf("error marshalling k0s config: %v", err)
}
files = append(files, cloudinit.File{
Path: "/etc/k0s.yaml",
Permissions: "0644",
Content: string(k0sConfigBytes),
})
config.Spec.Args = append(config.Spec.Args, "--config", "/etc/k0s.yaml")

// Reconcile the dynamic config
dErr := kutil.ReconcileDynamicConfig(ctx, cluster, c.Client, *config.Spec.K0s)
if dErr != nil {
Expand All @@ -209,6 +202,44 @@ func (c *ControlPlaneController) Reconcile(ctx context.Context, req ctrl.Request
}
}

currentKCPVersion, err := version.NewVersion(config.Spec.Version)
if err != nil {
return ctrl.Result{}, fmt.Errorf("error parsing k0s version: %w", err)
}
if currentKCPVersion.GreaterThanOrEqual(minVersionForETCDName) {
if config.Spec.K0s == nil {
config.Spec.K0s = &unstructured.Unstructured{
Object: make(map[string]interface{}),
}
}
// If it is not explicitly indicated to use Kine storage, we use the machine name to name the ETCD member.
kineStorage, found, err := unstructured.NestedString(config.Spec.K0s.Object, "spec", "storage", "kine", "dataSource")
if err != nil {
return ctrl.Result{}, fmt.Errorf("error retrieving storage.kine.datasource: %w", err)
}
if !found || kineStorage == "" {
err = unstructured.SetNestedMap(config.Spec.K0s.Object, map[string]interface{}{}, "spec", "storage", "etcd", "extraArgs")
if err != nil {
return ctrl.Result{}, fmt.Errorf("error ensuring intermediate maps spec.storage.etcd.extraArgs: %w", err)
}
err = unstructured.SetNestedField(config.Spec.K0s.Object, machine.Name, "spec", "storage", "etcd", "extraArgs", "name")
if err != nil {
return ctrl.Result{}, fmt.Errorf("error setting storage.etcd.extraArgs.name: %w", err)
}
}
}

k0sConfigBytes, err := config.Spec.K0s.MarshalJSON()
if err != nil {
return ctrl.Result{}, fmt.Errorf("error marshalling k0s config: %v", err)
}
files = append(files, cloudinit.File{
Path: "/etc/k0s.yaml",
Permissions: "0644",
Content: string(k0sConfigBytes),
})
config.Spec.Args = append(config.Spec.Args, "--config", "/etc/k0s.yaml")

if config.Status.Ready {
return ctrl.Result{}, nil
}
Expand Down
Loading