Skip to content

Commit f440ff1

Browse files
committed
fixup! feat: Nutanix VM image preflight check
* Refactor nutanixChecker to allow unit tests of all checks. * Add unit tests. * Fix field reported when spec init for worker spec fails. * Fix handling of credentials "Insecure" option. * Use lowercase 'v' in errors that refers to the Nutanix client
1 parent 06f8d8f commit f440ff1

File tree

9 files changed

+1579
-34
lines changed

9 files changed

+1579
-34
lines changed

pkg/webhook/preflight/nutanix/checker.go

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ func New(kclient ctrlclient.Client, cluster *clusterv1.Cluster) preflight.Checke
2424

2525
v3clientFactory: newV3Client,
2626
v4clientFactory: newV4Client,
27+
28+
vmImageCheckFunc: vmImageCheck,
29+
initNutanixConfigurationFunc: initNutanixConfiguration,
30+
initCredentialsCheckFunc: initCredentialsCheck,
31+
initVMImageChecksFunc: initVMImageChecks,
2732
}
2833
}
2934

@@ -42,6 +47,25 @@ type nutanixChecker struct {
4247
v4client v4client
4348
v4clientFactory func(prismgoclient.Credentials) (v4client, error)
4449

50+
vmImageCheckFunc func(
51+
n *nutanixChecker,
52+
machineDetails *carenv1.NutanixMachineDetails,
53+
field string,
54+
) preflight.Check
55+
56+
initNutanixConfigurationFunc func(
57+
n *nutanixChecker,
58+
) preflight.Check
59+
60+
initCredentialsCheckFunc func(
61+
ctx context.Context,
62+
n *nutanixChecker,
63+
) preflight.Check
64+
65+
initVMImageChecksFunc func(
66+
n *nutanixChecker,
67+
) []preflight.Check
68+
4569
log logr.Logger
4670
}
4771

@@ -53,11 +77,11 @@ func (n *nutanixChecker) Init(
5377
checks := []preflight.Check{
5478
// The configuration check must run first, because it initializes data used by all other checks,
5579
// and the credentials check second, because it initializes the Nutanix clients used by other checks.
56-
n.initNutanixConfiguration(),
57-
n.initCredentialsCheck(ctx),
80+
n.initNutanixConfigurationFunc(n),
81+
n.initCredentialsCheckFunc(ctx, n),
5882
}
5983

60-
checks = append(checks, n.initVMImageChecks()...)
84+
checks = append(checks, n.initVMImageChecksFunc(n)...)
6185

6286
// Add more checks here as needed.
6387

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright 2024 Nutanix. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package nutanix
5+
6+
import (
7+
"context"
8+
"fmt"
9+
"testing"
10+
11+
"github.com/stretchr/testify/assert"
12+
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
13+
14+
carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1"
15+
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight"
16+
)
17+
18+
func TestNutanixChecker_Init(t *testing.T) {
19+
tests := []struct {
20+
name string
21+
nutanixConfig *carenv1.NutanixClusterConfigSpec
22+
workerNodeConfigs map[string]*carenv1.NutanixWorkerNodeConfigSpec
23+
expectedCheckCount int
24+
expectedFirstCheckName string
25+
expectedSecondCheckName string
26+
vmImageCheckCount int
27+
}{
28+
{
29+
name: "basic initialization with no configs",
30+
nutanixConfig: nil,
31+
workerNodeConfigs: nil,
32+
expectedCheckCount: 2, // config check and credentials check
33+
expectedFirstCheckName: "NutanixConfiguration",
34+
expectedSecondCheckName: "NutanixCredentials",
35+
vmImageCheckCount: 0,
36+
},
37+
{
38+
name: "initialization with control plane config",
39+
nutanixConfig: &carenv1.NutanixClusterConfigSpec{
40+
ControlPlane: &carenv1.NutanixControlPlaneSpec{
41+
Nutanix: &carenv1.NutanixNodeSpec{},
42+
},
43+
},
44+
workerNodeConfigs: nil,
45+
expectedCheckCount: 3, // config check, credentials check, 1 VM image check
46+
expectedFirstCheckName: "NutanixConfiguration",
47+
expectedSecondCheckName: "NutanixCredentials",
48+
vmImageCheckCount: 1,
49+
},
50+
{
51+
name: "initialization with worker node configs",
52+
nutanixConfig: nil,
53+
workerNodeConfigs: map[string]*carenv1.NutanixWorkerNodeConfigSpec{
54+
"worker-1": {
55+
Nutanix: &carenv1.NutanixNodeSpec{},
56+
},
57+
"worker-2": {
58+
Nutanix: &carenv1.NutanixNodeSpec{},
59+
},
60+
},
61+
expectedCheckCount: 4, // config check, credentials check, 2 VM image checks
62+
expectedFirstCheckName: "NutanixConfiguration",
63+
expectedSecondCheckName: "NutanixCredentials",
64+
vmImageCheckCount: 2,
65+
},
66+
{
67+
name: "initialization with both control plane and worker node configs",
68+
nutanixConfig: &carenv1.NutanixClusterConfigSpec{
69+
ControlPlane: &carenv1.NutanixControlPlaneSpec{
70+
Nutanix: &carenv1.NutanixNodeSpec{},
71+
},
72+
},
73+
workerNodeConfigs: map[string]*carenv1.NutanixWorkerNodeConfigSpec{
74+
"worker-1": {
75+
Nutanix: &carenv1.NutanixNodeSpec{},
76+
},
77+
},
78+
expectedCheckCount: 4, // config check, credentials check, 2 VM image checks (1 CP + 1 worker)
79+
expectedFirstCheckName: "NutanixConfiguration",
80+
expectedSecondCheckName: "NutanixCredentials",
81+
vmImageCheckCount: 2,
82+
},
83+
}
84+
85+
for _, tt := range tests {
86+
t.Run(tt.name, func(t *testing.T) {
87+
// Create the checker
88+
checker := &nutanixChecker{
89+
cluster: &clusterv1.Cluster{},
90+
nutanixClusterConfigSpec: tt.nutanixConfig,
91+
nutanixWorkerNodeConfigSpecByMachineDeploymentName: tt.workerNodeConfigs,
92+
}
93+
94+
// Mock the sub-check functions to track their calls
95+
configCheckCalled := false
96+
credsCheckCalled := false
97+
vmImageCheckCount := 0
98+
99+
checker.initNutanixConfigurationFunc = func(n *nutanixChecker) preflight.Check {
100+
configCheckCalled = true
101+
return func(ctx context.Context) preflight.CheckResult {
102+
return preflight.CheckResult{
103+
Name: tt.expectedFirstCheckName,
104+
}
105+
}
106+
}
107+
108+
checker.initCredentialsCheckFunc = func(ctx context.Context, n *nutanixChecker) preflight.Check {
109+
credsCheckCalled = true
110+
return func(ctx context.Context) preflight.CheckResult {
111+
return preflight.CheckResult{
112+
Name: tt.expectedSecondCheckName,
113+
}
114+
}
115+
}
116+
117+
checker.initVMImageChecksFunc = func(n *nutanixChecker) []preflight.Check {
118+
checks := []preflight.Check{}
119+
for i := 0; i < tt.vmImageCheckCount; i++ {
120+
vmImageCheckCount++
121+
checks = append(checks, func(ctx context.Context) preflight.CheckResult {
122+
return preflight.CheckResult{
123+
Name: fmt.Sprintf("VMImageCheck-%d", i),
124+
}
125+
})
126+
}
127+
return checks
128+
}
129+
130+
// Call Init
131+
ctx := context.Background()
132+
checks := checker.Init(ctx)
133+
134+
// Verify the logger was set
135+
assert.NotNil(t, checker.log)
136+
137+
// Verify correct number of checks
138+
assert.Len(t, checks, tt.expectedCheckCount)
139+
140+
// Verify the sub-functions were called
141+
assert.True(t, configCheckCalled, "initNutanixConfiguration should have been called")
142+
assert.True(t, credsCheckCalled, "initCredentialsCheck should have been called")
143+
assert.Equal(t, tt.vmImageCheckCount, vmImageCheckCount, "Wrong number of VM image checks")
144+
145+
// Verify the first two checks when we have results
146+
if len(checks) >= 2 {
147+
result1 := checks[0](ctx)
148+
result2 := checks[1](ctx)
149+
assert.Equal(t, tt.expectedFirstCheckName, result1.Name)
150+
assert.Equal(t, tt.expectedSecondCheckName, result2.Name)
151+
}
152+
})
153+
}
154+
}

pkg/webhook/preflight/nutanix/clients.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// Copyright 2024 Nutanix. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
14
package nutanix
25

36
import (

pkg/webhook/preflight/nutanix/credentials.go

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ import (
1818

1919
const credentialsSecretDataKey = "credentials"
2020

21-
func (n *nutanixChecker) initCredentialsCheck(ctx context.Context) preflight.Check {
21+
func initCredentialsCheck(
22+
ctx context.Context,
23+
n *nutanixChecker,
24+
) preflight.Check {
2225
n.log.V(5).Info("Initializing Nutanix credentials check")
2326

2427
result := preflight.CheckResult{
@@ -142,23 +145,13 @@ func (n *nutanixChecker) initCredentialsCheck(ctx context.Context) preflight.Che
142145
}
143146
}
144147

145-
if !result.Allowed || result.Error {
146-
// If any error has happened, we should not try to initialize the credentials or clients, so we return early.
147-
return func(ctx context.Context) preflight.CheckResult {
148-
return result
149-
}
150-
}
151-
152148
// Initialize the credentials.
153149
n.credentials = prismgoclient.Credentials{
154150
Endpoint: fmt.Sprintf("%s:%d", host, port),
155151
URL: fmt.Sprintf("https://%s:%d", host, port),
156152
Username: usernamePassword.Username,
157153
Password: usernamePassword.Password,
158-
}
159-
if prismCentralEndpointSpec.Insecure {
160-
n.credentials.Insecure = true
161-
n.credentials.URL = fmt.Sprintf("http://%s:%d", host, port)
154+
Insecure: prismCentralEndpointSpec.Insecure,
162155
}
163156

164157
// Initialize the clients.
@@ -168,7 +161,7 @@ func (n *nutanixChecker) initCredentialsCheck(ctx context.Context) preflight.Che
168161
result.Error = true
169162
result.Causes = append(result.Causes,
170163
preflight.Cause{
171-
Message: fmt.Sprintf("failed to initialize Nutanix V4 client: %s", err),
164+
Message: fmt.Sprintf("failed to initialize Nutanix v4 client: %s", err),
172165
Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials",
173166
},
174167
)
@@ -180,21 +173,23 @@ func (n *nutanixChecker) initCredentialsCheck(ctx context.Context) preflight.Che
180173
result.Error = true
181174
result.Causes = append(result.Causes,
182175
preflight.Cause{
183-
Message: fmt.Sprintf("failed to initialize Nutanix V3 client: %s", err),
184-
Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials",
185-
},
186-
)
187-
}
188-
_, err = n.v3client.GetCurrentLoggedInUser(ctx)
189-
if err != nil {
190-
result.Allowed = false
191-
result.Error = true
192-
result.Causes = append(result.Causes,
193-
preflight.Cause{
194-
Message: fmt.Sprintf("failed to validate credentials using the v3 API client: %s", err),
176+
Message: fmt.Sprintf("failed to initialize Nutanix v3 client: %s", err),
195177
Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials",
196178
},
197179
)
180+
} else {
181+
// Validate the credentials using an API call.
182+
_, err = n.v3client.GetCurrentLoggedInUser(ctx)
183+
if err != nil {
184+
result.Allowed = false
185+
result.Error = true
186+
result.Causes = append(result.Causes,
187+
preflight.Cause{
188+
Message: fmt.Sprintf("failed to validate credentials using the v3 API client: %s", err),
189+
Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials",
190+
},
191+
)
192+
}
198193
}
199194

200195
return func(ctx context.Context) preflight.CheckResult {

0 commit comments

Comments
 (0)