Skip to content

Commit e215c8d

Browse files
committed
fixup! feat(preflight): Add a check for storage containers
Update for latest changes in base branch
1 parent 2638a37 commit e215c8d

File tree

5 files changed

+151
-183
lines changed

5 files changed

+151
-183
lines changed

pkg/webhook/preflight/nutanix/checker.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ import (
1818
)
1919

2020
var Checker = &nutanixChecker{
21-
configurationCheckFactory: newConfigurationCheck,
22-
credentialsCheckFactory: newCredentialsCheck,
23-
vmImageChecksFactory: newVMImageChecks,
21+
configurationCheckFactory: newConfigurationCheck,
22+
credentialsCheckFactory: newCredentialsCheck,
23+
vmImageChecksFactory: newVMImageChecks,
24+
storageContainerChecksFactory: newStorageContainerChecks,
2425
}
2526

2627
type nutanixChecker struct {
@@ -37,6 +38,10 @@ type nutanixChecker struct {
3738
vmImageChecksFactory func(
3839
cd *checkDependencies,
3940
) []preflight.Check
41+
42+
storageContainerChecksFactory func(
43+
cd *checkDependencies,
44+
) []preflight.Check
4045
}
4146

4247
type checkDependencies struct {
@@ -69,6 +74,7 @@ func (n *nutanixChecker) Init(
6974
}
7075

7176
checks = append(checks, n.vmImageChecksFactory(cd)...)
77+
checks = append(checks, n.storageContainerChecksFactory(cd)...)
7278

7379
// Add more checks here as needed.
7480

pkg/webhook/preflight/nutanix/checker_test.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -151,15 +151,18 @@ func TestNutanixChecker_Init(t *testing.T) {
151151
return checks
152152
}
153153

154-
checker.initStorageContainerChecksFunc = func(n *nutanixChecker) []preflight.Check {
154+
checker.storageContainerChecksFactory = func(cd *checkDependencies) []preflight.Check {
155155
checks := []preflight.Check{}
156156
for i := 0; i < tt.storageContainerCheckCount; i++ {
157157
storageContainerCheckCount++
158-
checks = append(checks, func(ctx context.Context) preflight.CheckResult {
159-
return preflight.CheckResult{
160-
Name: fmt.Sprintf("StorageContainerCheck-%d", i),
161-
}
162-
})
158+
checks = append(checks,
159+
&mockCheck{
160+
name: fmt.Sprintf("NutanixStorageContainer-%d", i),
161+
result: preflight.CheckResult{
162+
Allowed: true,
163+
},
164+
},
165+
)
163166
}
164167
return checks
165168
}

pkg/webhook/preflight/nutanix/clients.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,5 +163,3 @@ func (c *clientWrapper) ListStorageContainers(
163163
}
164164
return resp, nil
165165
}
166-
167-

pkg/webhook/preflight/nutanix/storagecontainer.go

Lines changed: 95 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -15,127 +15,128 @@ import (
1515
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight"
1616
)
1717

18-
func initStorageContainerChecks(n *nutanixChecker) []preflight.Check {
19-
checks := []preflight.Check{}
18+
const (
19+
csiParameterKeyStorageContainer = "storageContainer"
20+
)
2021

21-
// If there is no CSI configuration, there is no need to check for storage containers.
22-
if n.nutanixClusterConfigSpec == nil ||
23-
n.nutanixClusterConfigSpec.Addons == nil ||
24-
n.nutanixClusterConfigSpec.Addons.CSI == nil {
25-
return checks
22+
type storageContainerCheck struct {
23+
nodeSpec *carenv1.NutanixNodeSpec
24+
field string
25+
csiSpec *carenv1.CSIProvider
26+
nclient client
27+
}
28+
29+
func (c *storageContainerCheck) Name() string {
30+
return "NutanixStorageContainer"
31+
}
32+
33+
func (c *storageContainerCheck) Run(ctx context.Context) preflight.CheckResult {
34+
result := preflight.CheckResult{
35+
Allowed: true,
2636
}
2737

28-
if n.nutanixClusterConfigSpec != nil && n.nutanixClusterConfigSpec.ControlPlane != nil &&
29-
n.nutanixClusterConfigSpec.ControlPlane.Nutanix != nil {
30-
checks = append(checks,
31-
n.storageContainerCheckFunc(
32-
n,
33-
n.nutanixClusterConfigSpec.ControlPlane.Nutanix,
34-
"cluster.spec.topology[.name=clusterConfig].value.controlPlane.nutanix",
35-
&n.nutanixClusterConfigSpec.Addons.CSI.Providers.NutanixCSI,
38+
if c.csiSpec == nil {
39+
result.Allowed = false
40+
result.Error = true
41+
result.Causes = append(result.Causes, preflight.Cause{
42+
Message: fmt.Sprintf(
43+
"no storage container found for cluster %q",
44+
*c.nodeSpec.MachineDetails.Cluster.Name,
3645
),
37-
)
46+
Field: c.field,
47+
})
48+
49+
return result
3850
}
3951

40-
for mdName, nutanixWorkerNodeConfigSpec := range n.nutanixWorkerNodeConfigSpecByMachineDeploymentName {
41-
if nutanixWorkerNodeConfigSpec.Nutanix != nil {
42-
checks = append(checks,
43-
n.storageContainerCheckFunc(
44-
n,
45-
nutanixWorkerNodeConfigSpec.Nutanix,
46-
fmt.Sprintf(
47-
"cluster.spec.topology.workers.machineDeployments[.name=%s]"+
48-
".variables[.name=workerConfig].value.nutanix",
49-
mdName,
50-
),
51-
&n.nutanixClusterConfigSpec.Addons.CSI.Providers.NutanixCSI,
52-
),
53-
)
54-
}
52+
if c.csiSpec.StorageClassConfigs == nil {
53+
result.Allowed = false
54+
result.Causes = append(result.Causes, preflight.Cause{
55+
Message: fmt.Sprintf(
56+
"no storage class configs found for cluster %q",
57+
*c.nodeSpec.MachineDetails.Cluster.Name,
58+
),
59+
Field: c.field,
60+
})
61+
62+
return result
5563
}
5664

57-
return checks
58-
}
65+
for _, storageClassConfig := range c.csiSpec.StorageClassConfigs {
66+
if storageClassConfig.Parameters == nil {
67+
continue
68+
}
5969

60-
// storageContainerCheck checks if the storage container specified in the CSIProvider's StorageClassConfigs exists.
61-
// It admits the NodeSpec instead of the MachineDetails because the failure domains will be specified in the NodeSpec
62-
// and the MachineDetails.Cluster will be nil in that case.
63-
func storageContainerCheck(
64-
n *nutanixChecker,
65-
nodeSpec *carenv1.NutanixNodeSpec,
66-
field string,
67-
csiSpec *carenv1.CSIProvider,
68-
) preflight.Check {
69-
const (
70-
csiParameterKeyStorageContainer = "storageContainer"
71-
)
72-
73-
return func(ctx context.Context) preflight.CheckResult {
74-
result := preflight.CheckResult{
75-
Name: "NutanixStorageContainer",
76-
Allowed: true,
70+
storageContainer, ok := storageClassConfig.Parameters[csiParameterKeyStorageContainer]
71+
if !ok {
72+
continue
7773
}
78-
if csiSpec == nil {
74+
75+
// TODO: check if cluster name is set, if not use uuid.
76+
// If neither is set, use the cluster name from the NodeSpec failure domain.
77+
if _, err := getStorageContainer(c.nclient, c.nodeSpec, storageContainer); err != nil {
7978
result.Allowed = false
8079
result.Error = true
8180
result.Causes = append(result.Causes, preflight.Cause{
8281
Message: fmt.Sprintf(
83-
"no storage container found for cluster %q",
84-
*nodeSpec.MachineDetails.Cluster.Name,
82+
"failed to check if storage container named %q exists: %s",
83+
storageContainer,
84+
err,
8585
),
86-
Field: field,
86+
Field: c.field,
8787
})
8888

8989
return result
9090
}
91+
}
9192

92-
if csiSpec.StorageClassConfigs == nil {
93-
result.Allowed = false
94-
result.Causes = append(result.Causes, preflight.Cause{
95-
Message: fmt.Sprintf(
96-
"no storage class configs found for cluster %q",
97-
*nodeSpec.MachineDetails.Cluster.Name,
98-
),
99-
Field: field,
100-
})
93+
return result
94+
}
10195

102-
return result
103-
}
96+
func newStorageContainerChecks(cd *checkDependencies) []preflight.Check {
97+
checks := []preflight.Check{}
10498

105-
for _, storageClassConfig := range csiSpec.StorageClassConfigs {
106-
if storageClassConfig.Parameters == nil {
107-
continue
108-
}
109-
110-
storageContainer, ok := storageClassConfig.Parameters[csiParameterKeyStorageContainer]
111-
if !ok {
112-
continue
113-
}
114-
115-
// TODO: check if cluster name is set, if not use uuid.
116-
// If neither is set, use the cluster name from the NodeSpec failure domain.
117-
if _, err := getStorageContainer(n.v4client, nodeSpec, storageContainer); err != nil {
118-
result.Allowed = false
119-
result.Error = true
120-
result.Causes = append(result.Causes, preflight.Cause{
121-
Message: fmt.Sprintf(
122-
"failed to check if storage container named %q exists: %s",
123-
storageContainer,
124-
err,
125-
),
126-
Field: field,
127-
})
99+
// If there is no CSI configuration, there is no need to check for storage containers.
100+
if cd.nutanixClusterConfigSpec == nil ||
101+
cd.nutanixClusterConfigSpec.Addons == nil ||
102+
cd.nutanixClusterConfigSpec.Addons.CSI == nil {
103+
return checks
104+
}
128105

129-
return result
130-
}
131-
}
106+
if cd.nutanixClusterConfigSpec != nil && cd.nutanixClusterConfigSpec.ControlPlane != nil &&
107+
cd.nutanixClusterConfigSpec.ControlPlane.Nutanix != nil {
108+
checks = append(checks,
109+
&storageContainerCheck{
110+
nodeSpec: cd.nutanixClusterConfigSpec.ControlPlane.Nutanix,
111+
field: "cluster.spec.topology[.name=clusterConfig].value.controlPlane.nutanix",
112+
csiSpec: &cd.nutanixClusterConfigSpec.Addons.CSI.Providers.NutanixCSI,
113+
nclient: cd.nclient,
114+
},
115+
)
116+
}
132117

133-
return result
118+
for mdName, nutanixWorkerNodeConfigSpec := range cd.nutanixWorkerNodeConfigSpecByMachineDeploymentName {
119+
if nutanixWorkerNodeConfigSpec.Nutanix != nil {
120+
checks = append(checks,
121+
&storageContainerCheck{
122+
nodeSpec: nutanixWorkerNodeConfigSpec.Nutanix,
123+
field: fmt.Sprintf(
124+
"cluster.spec.topology.workers.machineDeployments[.name=%s]"+
125+
".variables[.name=workerConfig].value.nutanix",
126+
mdName,
127+
),
128+
csiSpec: &cd.nutanixClusterConfigSpec.Addons.CSI.Providers.NutanixCSI,
129+
nclient: cd.nclient,
130+
},
131+
)
132+
}
134133
}
134+
135+
return checks
135136
}
136137

137138
func getStorageContainer(
138-
client v4client,
139+
client client,
139140
nodeSpec *carenv1.NutanixNodeSpec,
140141
storageContainerName string,
141142
) (*clustermgmtv4.StorageContainer, error) {
@@ -175,7 +176,7 @@ func getStorageContainer(
175176
}
176177

177178
func getCluster(
178-
client v4client,
179+
client client,
179180
clusterIdentifier *v1beta1.NutanixResourceIdentifier,
180181
) (*clustermgmtv4.Cluster, error) {
181182
switch {

0 commit comments

Comments
 (0)