Skip to content

Commit ca0bc94

Browse files
dlipovetskydlipovetsky
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 ca0bc94

File tree

8 files changed

+1575
-34
lines changed

8 files changed

+1575
-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

‎pkg/webhook/preflight/nutanix/checker_test.go

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

‎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)