-
Notifications
You must be signed in to change notification settings - Fork 47
Conversation
Closes #918 Signed-off-by: knrt10 <kautilya@kinvolk.io>
Signed-off-by: knrt10 <kautilya@kinvolk.io>
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.
Make sense
func TestCommonControlPlaneChartsOrder(t *testing.T) { | ||
expectedOrder := []string{"pod-checkpointer", "kube-apiserver", "kubernetes", "calico", "lokomotive", "bootstrap-secrets"} //nolint:lll | ||
|
||
commonControlPlaneCharts := platform.CommonControlPlaneCharts() |
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.
Hooking on platform.CommonControlPlaneCharts is not the ideal place to check that, as the order may still be mangled by appending to the slice or even randomized before applying, but I think it's better than nothing.
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 understand that new charts might be appended in the list. Like it is don for kubelet chart. But I don't know if the order will change as you say so. I see it is all ordered from there on.
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.
The point is, we do not check right before the applying, so it still can be mangled somewhere.
@@ -45,3 +46,19 @@ func TestAppendVersionTag(t *testing.T) { | |||
t.Fatalf("should append version tag to existing map") | |||
} | |||
} | |||
|
|||
func TestCommonControlPlaneChartsOrder(t *testing.T) { | |||
expectedOrder := []string{"pod-checkpointer", "kube-apiserver", "kubernetes", "calico", "lokomotive", "bootstrap-secrets"} //nolint:lll |
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.
Topic for other issue probably, but most likely bootstrap-secrets
should be applied even before pod-checkpointer
, so nodes can join the cluster for pod-checkpointer
to converge.
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.
If we check if the worker nodes are up before applying bootstrap-secrets
it will still fail.
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 know, but that's a separate issue :)
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.
LGTM
Also, added test to check the order