Add machine names as etcd name #778
Conversation
apedriza
force-pushed
the
add_machine_name_as_etcd_name
branch
from
95c6840 to
d2bdb80
Compare
| return ctrl.Result{}, fmt.Errorf("error retrieving storage.etcd: %w", err) | ||
|
|
||
| } | ||
| if found && storage != nil { |
There was a problem hiding this comment.
We probably need to set it even if it's not found in the original config
There was a problem hiding this comment.
yes, I added a check to set it always if kind storage is not set
apedriza
force-pushed
the
add_machine_name_as_etcd_name
branch
3 times, most recently
from
95d6293 to
b631780
Compare
apedriza
force-pushed
the
add_machine_name_as_etcd_name
branch
from
b631780 to
a47ee0c
Compare
|
|
apedriza
force-pushed
the
add_machine_name_as_etcd_name
branch
from
a47ee0c to
195753f
Compare
internal/controller/bootstrap/controlplane_bootstrap_controller.go
Outdated
Show resolved
Hide resolved
internal/controller/bootstrap/controlplane_bootstrap_controller.go
Outdated
Show resolved
Hide resolved
| @@ -155,6 +156,25 @@ func (c *ControlPlaneController) Reconcile(ctx context.Context, req ctrl.Request | |||
| ) | |||
|
|
|||
| if config.Spec.K0s != nil { | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
- 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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Ive created an issue for that refactor #798
apedriza
force-pushed
the
add_machine_name_as_etcd_name
branch
from
195753f to
110c116
Compare
apedriza
force-pushed
the
add_machine_name_as_etcd_name
branch
from
110c116 to
e7e447e
Compare
Signed-off-by: Adrian Pedriza <adripedriza@gmail.com>
apedriza
force-pushed
the
add_machine_name_as_etcd_name
branch
from
e7e447e to
10f2faa
Compare
Partially fix #768