Skip to content

Commit f8d2dce

Browse files
dkoshkinjimmidyson
dkoshkin
and
jimmidyson
authored
fix: add omitempty to addon strategy (#795)
**What problem does this PR solve?**: Saw a failure when using the API to generate a spec and not setting the `strategy` in Go code: ``` is invalid: [spec.topology.variables.addons.cni.strategy: Unsupported value: "": supported values: "ClusterResourceSet", "HelmAddon"... ``` Adding `+kubebuilder:validation:Optional` didn't change the generated CRDs, but I think we still want them? **Which issue(s) this PR fixes**: Fixes # **How Has This Been Tested?**: <!-- Please describe the tests that you ran to verify your changes. Provide output from the tests and any manual steps needed to replicate the tests. --> **Special notes for your reviewer**: <!-- Use this to provide any additional information to the reviewers. This may include: - Best way to review the PR. - Where the author wants the most review attention on. - etc. --> --------- Co-authored-by: Jimmi Dyson <[email protected]>
1 parent 7d69360 commit f8d2dce

23 files changed

+150
-103
lines changed

‎api/v1alpha1/addon_types.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ type GenericAddons struct {
9696
ServiceLoadBalancer *ServiceLoadBalancer `json:"serviceLoadBalancer,omitempty"`
9797
}
9898

99-
// +kubebuilder:validation:Required
99+
// +kubebuilder:validation:Optional
100100
// +kubebuilder:validation:Enum=ClusterResourceSet;HelmAddon
101101
type AddonStrategy string
102102

@@ -109,22 +109,22 @@ type CNI struct {
109109

110110
// Addon strategy used to deploy the CNI provider to the workload cluster.
111111
// +kubebuilder:default=HelmAddon
112-
Strategy AddonStrategy `json:"strategy"`
112+
Strategy *AddonStrategy `json:"strategy,omitempty"`
113113
}
114114

115115
// NFD tells us to enable or disable the node feature discovery addon.
116116
type NFD struct {
117117
// Addon strategy used to deploy Node Feature Discovery (NFD) to the workload cluster.
118118
// +kubebuilder:default=HelmAddon
119-
Strategy AddonStrategy `json:"strategy"`
119+
Strategy *AddonStrategy `json:"strategy,omitempty"`
120120
}
121121

122122
// ClusterAutoscaler tells us to enable or disable the cluster-autoscaler addon.
123123
type ClusterAutoscaler struct {
124124
// Addon strategy used to deploy cluster-autoscaler to the management cluster
125125
// targeting the workload cluster.
126126
// +kubebuilder:default=HelmAddon
127-
Strategy AddonStrategy `json:"strategy"`
127+
Strategy *AddonStrategy `json:"strategy,omitempty"`
128128
}
129129

130130
type GenericCSI struct {
@@ -139,7 +139,7 @@ type GenericCSI struct {
139139
type SnapshotController struct {
140140
// Addon strategy used to deploy the snapshot controller to the workload cluster.
141141
// +kubebuilder:default=HelmAddon
142-
Strategy AddonStrategy `json:"strategy"`
142+
Strategy *AddonStrategy `json:"strategy,omitempty"`
143143
}
144144

145145
type DefaultStorage struct {
@@ -197,7 +197,7 @@ type CSIProvider struct {
197197

198198
// Addon strategy used to deploy the CSI provider to the workload cluster.
199199
// +kubebuilder:default=HelmAddon
200-
Strategy AddonStrategy `json:"strategy"`
200+
Strategy *AddonStrategy `json:"strategy,omitempty"`
201201

202202
// The reference to any secret used by the CSI Provider.
203203
// +kubebuilder:validation:Optional
@@ -239,7 +239,7 @@ type CCM struct {
239239

240240
// Addon strategy used to deploy the CCM to the workload cluster.
241241
// +kubebuilder:default=HelmAddon
242-
Strategy AddonStrategy `json:"strategy"`
242+
Strategy *AddonStrategy `json:"strategy,omitempty"`
243243
}
244244

245245
type CCMCredentials struct {

‎api/v1alpha1/crds/caren.nutanix.com_awsclusterconfigs.yaml

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,6 @@ spec:
7171
- ClusterResourceSet
7272
- HelmAddon
7373
type: string
74-
required:
75-
- strategy
7674
type: object
7775
clusterAutoscaler:
7876
description: ClusterAutoscaler tells us to enable or disable the cluster-autoscaler addon.
@@ -86,8 +84,6 @@ spec:
8684
- ClusterResourceSet
8785
- HelmAddon
8886
type: string
89-
required:
90-
- strategy
9187
type: object
9288
cni:
9389
description: CNI required for providing CNI configuration.
@@ -107,7 +103,6 @@ spec:
107103
type: string
108104
required:
109105
- provider
110-
- strategy
111106
type: object
112107
csi:
113108
properties:
@@ -187,7 +182,6 @@ spec:
187182
type: string
188183
required:
189184
- storageClassConfigs
190-
- strategy
191185
type: object
192186
required:
193187
- aws-ebs
@@ -202,8 +196,6 @@ spec:
202196
- ClusterResourceSet
203197
- HelmAddon
204198
type: string
205-
required:
206-
- strategy
207199
type: object
208200
required:
209201
- defaultStorage
@@ -219,8 +211,6 @@ spec:
219211
- ClusterResourceSet
220212
- HelmAddon
221213
type: string
222-
required:
223-
- strategy
224214
type: object
225215
serviceLoadBalancer:
226216
properties:

‎api/v1alpha1/crds/caren.nutanix.com_dockerclusterconfigs.yaml

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,6 @@ spec:
7171
- ClusterResourceSet
7272
- HelmAddon
7373
type: string
74-
required:
75-
- strategy
7674
type: object
7775
clusterAutoscaler:
7876
description: ClusterAutoscaler tells us to enable or disable the cluster-autoscaler addon.
@@ -86,8 +84,6 @@ spec:
8684
- ClusterResourceSet
8785
- HelmAddon
8886
type: string
89-
required:
90-
- strategy
9187
type: object
9288
cni:
9389
description: CNI required for providing CNI configuration.
@@ -107,7 +103,6 @@ spec:
107103
type: string
108104
required:
109105
- provider
110-
- strategy
111106
type: object
112107
csi:
113108
properties:
@@ -187,7 +182,6 @@ spec:
187182
type: string
188183
required:
189184
- storageClassConfigs
190-
- strategy
191185
type: object
192186
required:
193187
- local-path
@@ -202,8 +196,6 @@ spec:
202196
- ClusterResourceSet
203197
- HelmAddon
204198
type: string
205-
required:
206-
- strategy
207199
type: object
208200
required:
209201
- defaultStorage
@@ -219,8 +211,6 @@ spec:
219211
- ClusterResourceSet
220212
- HelmAddon
221213
type: string
222-
required:
223-
- strategy
224214
type: object
225215
serviceLoadBalancer:
226216
properties:

‎api/v1alpha1/crds/caren.nutanix.com_nutanixclusterconfigs.yaml

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,6 @@ spec:
7171
- ClusterResourceSet
7272
- HelmAddon
7373
type: string
74-
required:
75-
- strategy
7674
type: object
7775
clusterAutoscaler:
7876
description: ClusterAutoscaler tells us to enable or disable the cluster-autoscaler addon.
@@ -86,8 +84,6 @@ spec:
8684
- ClusterResourceSet
8785
- HelmAddon
8886
type: string
89-
required:
90-
- strategy
9187
type: object
9288
cni:
9389
description: CNI required for providing CNI configuration.
@@ -107,7 +103,6 @@ spec:
107103
type: string
108104
required:
109105
- provider
110-
- strategy
111106
type: object
112107
csi:
113108
properties:
@@ -187,7 +182,6 @@ spec:
187182
type: string
188183
required:
189184
- storageClassConfigs
190-
- strategy
191185
type: object
192186
required:
193187
- nutanix
@@ -202,8 +196,6 @@ spec:
202196
- ClusterResourceSet
203197
- HelmAddon
204198
type: string
205-
required:
206-
- strategy
207199
type: object
208200
required:
209201
- defaultStorage
@@ -219,8 +211,6 @@ spec:
219211
- ClusterResourceSet
220212
- HelmAddon
221213
type: string
222-
required:
223-
- strategy
224214
type: object
225215
serviceLoadBalancer:
226216
properties:

‎api/v1alpha1/zz_generated.deepcopy.go

Lines changed: 37 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/handlers/generic/lifecycle/ccm/aws/handler.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/go-logr/logr"
1111
"github.com/spf13/pflag"
12+
"k8s.io/utils/ptr"
1213
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
1314
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
1415

@@ -91,7 +92,7 @@ func (a *AWSCCM) Apply(
9192
}
9293

9394
var strategy addons.Applier
94-
switch clusterConfig.Addons.CCM.Strategy {
95+
switch ptr.Deref(clusterConfig.Addons.CCM.Strategy, "") {
9596
case v1alpha1.AddonStrategyHelmAddon:
9697
helmChart, err := a.helmChartInfoGetter.For(ctx, log, config.AWSCCM)
9798
if err != nil {
@@ -109,8 +110,10 @@ func (a *AWSCCM) Apply(
109110
},
110111
client: a.client,
111112
}
113+
case "":
114+
return fmt.Errorf("strategy not specified for AWS CCM")
112115
default:
113-
return fmt.Errorf("strategy %s not implemented", clusterConfig.Addons.CCM.Strategy)
116+
return fmt.Errorf("strategy %s not implemented", *clusterConfig.Addons.CCM.Strategy)
114117
}
115118

116119
if err := strategy.Apply(ctx, cluster, a.config.DefaultsNamespace(), log); err != nil {

0 commit comments

Comments
 (0)