Skip to content

Commit 915c303

Browse files
arturshadnikTylerGillson
arturshadnik
and
TylerGillson
authored
✨ implement addon enable/disable flow (#33)
* 🐛 ignore hub apiserver override when forcing internal endpoint lookup (#34) * fix: ignore hub apiserver when force-internal-endpoint-lookup is set; move hub CA to spec.hub * feat: add timeout, verbosity to API; modernize applicable omitemptys * feat: add timeout, verbosity args to all clusteradm commands * fix: handle stdout & stderr separately * chore: fix 'make test' * ci: fix E2E artifact paths * fix: handle approval of duplicate CSRs --------- Signed-off-by: Tyler Gillson <[email protected]> * feat: implement addon enable/disable flow Signed-off-by: Artur Shad Nik <[email protected]> * chore: add hub and spoke addon conditions Signed-off-by: Artur Shad Nik <[email protected]> * feat: add validation to prevent deleting in-use addons Signed-off-by: Artur Shad Nik <[email protected]> * test: add new expected condition Signed-off-by: Artur Shad Nik <[email protected]> * wip: e2e test [skip ci] Signed-off-by: Artur Shad Nik <[email protected]> * test: add e2e tests Signed-off-by: Artur Shad Nik <[email protected]> * add docstrings; move code to helper to address gocyclo Signed-off-by: Artur Shad Nik <[email protected]> * chore: trigger ci Signed-off-by: Artur Shad Nik <[email protected]> * chore: trigger ci Signed-off-by: Artur Shad Nik <[email protected]> * chore: demistify magic numbers Signed-off-by: Artur Shad Nik <[email protected]> * feat: check number of MW against number of MCAO Signed-off-by: Artur Shad Nik <[email protected]> * chore: helper for max type length Signed-off-by: Artur Shad Nik <[email protected]> * fix: delete unused code Signed-off-by: Artur Shad Nik <[email protected]> * chore: address review comments Signed-off-by: Artur Shad Nik <[email protected]> * chore: update magic number; use ownerRef checks when deleting spoke Signed-off-by: Artur Shad Nik <[email protected]> * fix: make unjoined check more robust Signed-off-by: Artur Shad Nik <[email protected]> * chore: reorder checks Signed-off-by: Artur Shad Nik <[email protected]> * chore: add unit tests; handle unknown conditions Signed-off-by: Artur Shad Nik <[email protected]> --------- Signed-off-by: Tyler Gillson <[email protected]> Signed-off-by: Artur Shad Nik <[email protected]> Co-authored-by: Tyler Gillson <[email protected]>
1 parent d62ba72 commit 915c303

File tree

18 files changed

+767
-48
lines changed

18 files changed

+767
-48
lines changed

‎fleetconfig-controller/api/v1alpha1/constants.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,14 @@ const (
1010
// FleetConfigHubInitialized means that the Hub has been initialized.
1111
FleetConfigHubInitialized = "HubInitialized"
1212

13+
// FleetConfigAddonsConfigured means that all addons have been configured on the Hub.
14+
FleetConfigAddonsConfigured = "AddonsConfigured"
15+
1316
// FleetConfigCleanupFailed means that a failure occurred during cleanup.
1417
FleetConfigCleanupFailed = "CleanupFailed"
18+
19+
// FleetConfigAddonsEnabled means that all addons have been enabled for a particular Spoke.
20+
FleetConfigAddonsEnabled = "AddonsEnabled"
1521
)
1622

1723
// FleetConfig condition reasons

‎fleetconfig-controller/api/v1alpha1/fleetconfig_types.go‎

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -392,13 +392,20 @@ func (s *Spoke) GetPurgeKlusterletOperator() bool {
392392

393393
// JoinType returns a status condition type indicating that a particular Spoke cluster has joined the Hub.
394394
func (s *Spoke) JoinType() string {
395-
return fmt.Sprintf("spoke-cluster-%s-joined", s.conditionName())
395+
baseMsg := "spoke-cluster-%s-joined"
396+
return fmt.Sprintf(baseMsg, s.conditionName(maxConditionNameLen(baseMsg)))
396397
}
397398

398-
func (s *Spoke) conditionName() string {
399+
// AddonEnableType returns a status condition type indicating that a particular Spoke cluster's addons have been disabled.
400+
func (s *Spoke) AddonEnableType() string {
401+
baseMsg := "spoke-cluster-%s-addons-enabled"
402+
return fmt.Sprintf(baseMsg, s.conditionName(maxConditionNameLen(baseMsg)))
403+
}
404+
405+
func (s *Spoke) conditionName(c int) string {
399406
name := s.Name
400-
if len(name) > 42 {
401-
name = name[:42] // account for extra 21 chars in the condition type (max. total of 63)
407+
if len(name) > c {
408+
name = name[:c] // account for extra chars in the condition type (max. total of 63)
402409
}
403410
return name
404411
}
@@ -431,6 +438,11 @@ type JoinedSpoke struct {
431438
// +kubebuilder:default:=true
432439
// +optional
433440
PurgeKlusterletOperator bool `json:"purgeKlusterletOperator,omitempty"`
441+
442+
// EnabledAddons is the list of addons that are currently enabled for the cluster.
443+
// +kubebuilder:default:={}
444+
// +optional
445+
EnabledAddons []string `json:"enabledAddons,omitempty"`
434446
}
435447

436448
// GetName returns the name of the joined spoke cluster.
@@ -450,13 +462,20 @@ func (j *JoinedSpoke) GetPurgeKlusterletOperator() bool {
450462

451463
// UnjoinType returns a status condition type indicating that a particular Spoke cluster has been removed from the Hub.
452464
func (j *JoinedSpoke) UnjoinType() string {
453-
return fmt.Sprintf("spoke-cluster-%s-unjoined", j.conditionName())
465+
baseMsg := "spoke-cluster-%s-unjoined"
466+
return fmt.Sprintf(baseMsg, j.conditionName(maxConditionNameLen(baseMsg)))
454467
}
455468

456-
func (j *JoinedSpoke) conditionName() string {
469+
// AddonDisableType returns a status condition type indicating that a particular Spoke cluster's addons have been disabled.
470+
func (j *JoinedSpoke) AddonDisableType() string {
471+
baseMsg := "spoke-cluster-%s-addons-disabled"
472+
return fmt.Sprintf(baseMsg, j.conditionName(maxConditionNameLen(baseMsg)))
473+
}
474+
475+
func (j *JoinedSpoke) conditionName(c int) string {
457476
name := j.Name
458-
if len(name) > 40 {
459-
name = name[:40] // account for extra 23 chars in the condition type (max. total of 63)
477+
if len(name) > c {
478+
name = name[:c] // account for extra chars in the condition type (max. total of 63)
460479
}
461480
return name
462481
}
@@ -656,3 +675,30 @@ type FleetConfigList struct {
656675
func init() {
657676
SchemeBuilder.Register(&FleetConfig{}, &FleetConfigList{})
658677
}
678+
679+
func maxConditionNameLen(base string) int {
680+
maxLen := 316 // a metav1.Condition.Type type can be at most 316 chars long
681+
return maxLen - (len(base) - 2)
682+
}
683+
684+
// IsUnjoined returns true if a spoke cluster was successfully joined and then successfully unjoined, and the unjoined timestamp is after the joined timestamp.
685+
// Returns false in all other cases.
686+
func (m *FleetConfig) IsUnjoined(spoke Spoke, joinedSpoke JoinedSpoke) bool {
687+
joinedC := m.GetCondition(spoke.JoinType())
688+
if joinedC == nil {
689+
return false
690+
}
691+
unjoinedC := m.GetCondition(joinedSpoke.UnjoinType())
692+
if unjoinedC == nil {
693+
return false
694+
}
695+
if unjoinedC.Status != metav1.ConditionTrue {
696+
return false
697+
}
698+
// at this point, unjoined is known to be true. if joined is not true, cluster has not been successfully rejoined
699+
if joinedC.Status != metav1.ConditionTrue {
700+
return true
701+
}
702+
// if both exist and are true, compare timestamps
703+
return unjoinedC.LastTransitionTime.After(joinedC.LastTransitionTime.Time)
704+
}

‎fleetconfig-controller/api/v1alpha1/fleetconfig_types_test.go‎

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
package v1alpha1
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8+
)
9+
10+
func TestFleetConfig_IsUnjoined(t *testing.T) {
11+
// Create base time for testing
12+
baseTime := time.Now()
13+
joinedTime := metav1.NewTime(baseTime)
14+
unjoinedTime := metav1.NewTime(baseTime.Add(time.Hour)) // 1 hour later
15+
16+
tests := []struct {
17+
name string
18+
fleetConfig *FleetConfig
19+
spoke Spoke
20+
joinedSpoke JoinedSpoke
21+
want bool
22+
}{
23+
{
24+
name: "successfully joined and unjoined - should return true",
25+
fleetConfig: &FleetConfig{
26+
Status: FleetConfigStatus{
27+
Conditions: []Condition{
28+
{
29+
Condition: metav1.Condition{
30+
Type: "spoke-cluster-testcluster-joined",
31+
Status: metav1.ConditionTrue,
32+
LastTransitionTime: joinedTime,
33+
},
34+
},
35+
{
36+
Condition: metav1.Condition{
37+
Type: "spoke-cluster-testcluster-unjoined",
38+
Status: metav1.ConditionTrue,
39+
LastTransitionTime: unjoinedTime,
40+
},
41+
},
42+
},
43+
},
44+
},
45+
spoke: Spoke{Name: "testcluster"},
46+
joinedSpoke: JoinedSpoke{Name: "testcluster"},
47+
want: true,
48+
},
49+
{
50+
name: "joined but not unjoined - should return false",
51+
fleetConfig: &FleetConfig{
52+
Status: FleetConfigStatus{
53+
Conditions: []Condition{
54+
{
55+
Condition: metav1.Condition{
56+
Type: "spoke-cluster-testcluster-joined",
57+
Status: metav1.ConditionTrue,
58+
LastTransitionTime: joinedTime,
59+
},
60+
},
61+
// No unjoin condition
62+
},
63+
},
64+
},
65+
spoke: Spoke{Name: "testcluster"},
66+
joinedSpoke: JoinedSpoke{Name: "testcluster"},
67+
want: false,
68+
},
69+
{
70+
name: "both conditions exist but unjoin is False - should return false",
71+
fleetConfig: &FleetConfig{
72+
Status: FleetConfigStatus{
73+
Conditions: []Condition{
74+
{
75+
Condition: metav1.Condition{
76+
Type: "spoke-cluster-testcluster-joined",
77+
Status: metav1.ConditionTrue,
78+
LastTransitionTime: joinedTime,
79+
},
80+
},
81+
{
82+
Condition: metav1.Condition{
83+
Type: "spoke-cluster-testcluster-unjoined",
84+
Status: metav1.ConditionFalse,
85+
LastTransitionTime: unjoinedTime,
86+
},
87+
},
88+
},
89+
},
90+
},
91+
spoke: Spoke{Name: "testcluster"},
92+
joinedSpoke: JoinedSpoke{Name: "testcluster"},
93+
want: false,
94+
},
95+
{
96+
name: "both conditions exist but join is False - should return true",
97+
fleetConfig: &FleetConfig{
98+
Status: FleetConfigStatus{
99+
Conditions: []Condition{
100+
{
101+
Condition: metav1.Condition{
102+
Type: "spoke-cluster-testcluster-joined",
103+
Status: metav1.ConditionFalse,
104+
LastTransitionTime: metav1.NewTime(unjoinedTime.Add(time.Hour)), // 1 hour after unjoin, failed re-join
105+
},
106+
},
107+
{
108+
Condition: metav1.Condition{
109+
Type: "spoke-cluster-testcluster-unjoined",
110+
Status: metav1.ConditionTrue,
111+
LastTransitionTime: unjoinedTime,
112+
},
113+
},
114+
},
115+
},
116+
},
117+
spoke: Spoke{Name: "testcluster"},
118+
joinedSpoke: JoinedSpoke{Name: "testcluster"},
119+
want: true,
120+
},
121+
{
122+
name: "both conditions exist but unjoin time is before join time - should return false",
123+
fleetConfig: &FleetConfig{
124+
Status: FleetConfigStatus{
125+
Conditions: []Condition{
126+
{
127+
Condition: metav1.Condition{
128+
Type: "spoke-cluster-testcluster-joined",
129+
Status: metav1.ConditionTrue,
130+
LastTransitionTime: joinedTime,
131+
},
132+
},
133+
{
134+
Condition: metav1.Condition{
135+
Type: "spoke-cluster-testcluster-unjoined",
136+
Status: metav1.ConditionTrue,
137+
LastTransitionTime: metav1.NewTime(baseTime.Add(-time.Hour)), // 1 hour earlier
138+
},
139+
},
140+
},
141+
},
142+
},
143+
spoke: Spoke{Name: "testcluster"},
144+
joinedSpoke: JoinedSpoke{Name: "testcluster"},
145+
want: false,
146+
},
147+
{
148+
name: "both conditions exist with same time - should return false",
149+
fleetConfig: &FleetConfig{
150+
Status: FleetConfigStatus{
151+
Conditions: []Condition{
152+
{
153+
Condition: metav1.Condition{
154+
Type: "spoke-cluster-testcluster-joined",
155+
Status: metav1.ConditionTrue,
156+
LastTransitionTime: joinedTime,
157+
},
158+
},
159+
{
160+
Condition: metav1.Condition{
161+
Type: "spoke-cluster-testcluster-unjoined",
162+
Status: metav1.ConditionTrue,
163+
LastTransitionTime: joinedTime, // Same time
164+
},
165+
},
166+
},
167+
},
168+
},
169+
spoke: Spoke{Name: "testcluster"},
170+
joinedSpoke: JoinedSpoke{Name: "testcluster"},
171+
want: false,
172+
},
173+
{
174+
name: "no conditions exist - should return false",
175+
fleetConfig: &FleetConfig{
176+
Status: FleetConfigStatus{
177+
Conditions: []Condition{},
178+
},
179+
},
180+
spoke: Spoke{Name: "testcluster"},
181+
joinedSpoke: JoinedSpoke{Name: "testcluster"},
182+
want: false,
183+
},
184+
{
185+
name: "conditions with Unknown status - should return false",
186+
fleetConfig: &FleetConfig{
187+
Status: FleetConfigStatus{
188+
Conditions: []Condition{
189+
{
190+
Condition: metav1.Condition{
191+
Type: "spoke-cluster-testcluster-joined",
192+
Status: metav1.ConditionUnknown,
193+
LastTransitionTime: joinedTime,
194+
},
195+
},
196+
{
197+
Condition: metav1.Condition{
198+
Type: "spoke-cluster-testcluster-unjoined",
199+
Status: metav1.ConditionTrue,
200+
LastTransitionTime: unjoinedTime,
201+
},
202+
},
203+
},
204+
},
205+
},
206+
spoke: Spoke{Name: "testcluster"},
207+
joinedSpoke: JoinedSpoke{Name: "testcluster"},
208+
want: true,
209+
},
210+
}
211+
212+
for _, tt := range tests {
213+
t.Run(tt.name, func(t *testing.T) {
214+
got := tt.fleetConfig.IsUnjoined(tt.spoke, tt.joinedSpoke)
215+
if got != tt.want {
216+
t.Errorf("FleetConfig.IsUnjoined() = %v, want %v", got, tt.want)
217+
}
218+
})
219+
}
220+
}

‎fleetconfig-controller/api/v1alpha1/fleetconfig_webhook.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,8 @@ func (v *FleetConfigCustomValidator) ValidateCreate(ctx context.Context, obj run
130130
}
131131
}
132132

133-
allErrs = append(allErrs, validateAddonConfigs(ctx, v.client, fc)...)
134-
133+
allErrs = append(allErrs, validateAddonConfigs(ctx, v.client, nil, fc)...)
134+
allErrs = append(allErrs, validateAddons(fc)...)
135135
if len(allErrs) > 0 {
136136
return warnings, errors.NewInvalid(GroupKind, fc.Name, allErrs)
137137
}
@@ -166,7 +166,8 @@ func (v *FleetConfigCustomValidator) ValidateUpdate(ctx context.Context, oldObj,
166166
return nil, err
167167
}
168168

169-
errs := validateAddonConfigs(ctx, v.client, fc)
169+
errs := validateAddonConfigs(ctx, v.client, oldFc, fc)
170+
errs = append(errs, validateAddons(fc)...)
170171
if len(errs) > 0 {
171172
return nil, errors.NewInvalid(GroupKind, fc.Name, errs)
172173
}

0 commit comments

Comments
 (0)