Skip to content

Commit 174e8ce

Browse files
dlipovetskydlipovetsky
committed
fixup! feat: Nutanix VM image preflight check
Run VM Image check only if Nutanix clients are initialized. This avoids returning errors that are not actionable. The user will see an error from the credentials check only.
1 parent bb6e5e0 commit 174e8ce

File tree

5 files changed

+62
-63
lines changed

5 files changed

+62
-63
lines changed

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ type nutanixChecker struct {
3939
nutanixClusterConfigSpec *carenv1.NutanixClusterConfigSpec
4040
nutanixWorkerNodeConfigSpecByMachineDeploymentName map[string]*carenv1.NutanixWorkerNodeConfigSpec
4141

42-
credentials prismgoclient.Credentials
43-
4442
v3client v3client
4543
v3clientFactory func(prismgoclient.Credentials) (v3client, error)
4644

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

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ func initCredentialsCheck(
146146
}
147147

148148
// Initialize the credentials.
149-
n.credentials = prismgoclient.Credentials{
149+
credentials := prismgoclient.Credentials{
150150
Endpoint: fmt.Sprintf("%s:%d", host, port),
151151
URL: fmt.Sprintf("https://%s:%d", host, port),
152152
Username: usernamePassword.Username,
@@ -155,7 +155,7 @@ func initCredentialsCheck(
155155
}
156156

157157
// Initialize the clients.
158-
n.v4client, err = n.v4clientFactory(n.credentials)
158+
v4client, err := n.v4clientFactory(credentials)
159159
if err != nil {
160160
result.Allowed = false
161161
result.Error = true
@@ -167,7 +167,7 @@ func initCredentialsCheck(
167167
)
168168
}
169169

170-
n.v3client, err = n.v3clientFactory(n.credentials)
170+
v3client, err := n.v3clientFactory(credentials)
171171
if err != nil {
172172
result.Allowed = false
173173
result.Error = true
@@ -177,21 +177,35 @@ func initCredentialsCheck(
177177
Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials",
178178
},
179179
)
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-
)
180+
}
181+
182+
if v3client == nil || v4client == nil {
183+
return func(ctx context.Context) preflight.CheckResult {
184+
return result
192185
}
193186
}
194187

188+
// Validate the credentials using an API call.
189+
_, err = v3client.GetCurrentLoggedInUser(ctx)
190+
if err != nil {
191+
result.Allowed = false
192+
result.Error = true
193+
result.Causes = append(result.Causes,
194+
preflight.Cause{
195+
Message: fmt.Sprintf("Failed to validate credentials using the v3 API client. "+
196+
"The URL and/or credentials may be incorrect. (Error: %q)", err),
197+
Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint",
198+
},
199+
)
200+
return func(ctx context.Context) preflight.CheckResult {
201+
return result
202+
}
203+
}
204+
205+
// We initialized both clients, and verified the credentials using the v3 client.
206+
n.v3client = v3client
207+
n.v4client = v4client
208+
195209
return func(ctx context.Context) preflight.CheckResult {
196210
return result
197211
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ func TestInitCredentialsCheck_FailedToGetCurrentLoggedInUser(t *testing.T) {
174174
result := check(context.Background())
175175
assert.False(t, result.Allowed)
176176
assert.True(t, result.Error)
177-
assert.Contains(t, result.Causes[0].Message, "failed to validate credentials using the v3 API client")
177+
assert.Contains(t, result.Causes[0].Message, "Failed to validate credentials using the v3 API client.")
178178
}
179179

180180
func validNutanixChecker() *nutanixChecker {

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

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ func initVMImageChecks(
1919
) []preflight.Check {
2020
checks := []preflight.Check{}
2121

22+
if n.v4client == nil {
23+
return checks
24+
}
25+
2226
if n.nutanixClusterConfigSpec != nil && n.nutanixClusterConfigSpec.ControlPlane != nil &&
2327
n.nutanixClusterConfigSpec.ControlPlane.Nutanix != nil {
2428
checks = append(checks,
@@ -62,19 +66,6 @@ func vmImageCheck(
6266
Allowed: false,
6367
}
6468

65-
// If the v4 client is not initialized, we cannot perform VM image checks.
66-
if n.v4client == nil {
67-
result.Allowed = false
68-
result.Error = true
69-
result.Causes = append(result.Causes,
70-
preflight.Cause{
71-
Message: "Nutanix v4 client is not initialized, cannot perform VM image checks",
72-
Field: "",
73-
},
74-
)
75-
return result
76-
}
77-
7869
if machineDetails.ImageLookup != nil {
7970
result.Allowed = true
8071
result.Warnings = append(

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

Lines changed: 28 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -59,27 +59,6 @@ func TestVMImageCheck(t *testing.T) {
5959
machineDetails *carenv1.NutanixMachineDetails
6060
want preflight.CheckResult
6161
}{
62-
{
63-
name: "Nutanix v4 client not initialized",
64-
v4client: nil,
65-
machineDetails: &carenv1.NutanixMachineDetails{
66-
Image: &capxv1.NutanixResourceIdentifier{
67-
Type: capxv1.NutanixIdentifierUUID,
68-
UUID: ptr.To("test-uuid"),
69-
},
70-
},
71-
want: preflight.CheckResult{
72-
Name: "NutanixVMImage",
73-
Allowed: false,
74-
Error: true,
75-
Causes: []preflight.Cause{
76-
{
77-
Message: "Nutanix v4 client is not initialized, cannot perform VM image checks",
78-
Field: "",
79-
},
80-
},
81-
},
82-
},
8362
{
8463
name: "imageLookup not yet supported",
8564
v4client: &mockV4Client{},
@@ -514,16 +493,27 @@ func TestInitVMImageChecks(t *testing.T) {
514493
name string
515494
nutanixClusterConfigSpec *carenv1.NutanixClusterConfigSpec
516495
nutanixWorkerNodeConfigSpecByMDName map[string]*carenv1.NutanixWorkerNodeConfigSpec
496+
v4client v4client
517497
expectedChecks int
518498
expectedControlPlaneCheckFieldIncluded bool
519499
expectedWorkerNodeCheckFieldPatternExists bool
520500
}{
521501
{
522-
name: "no nutanix configuration",
523-
nutanixClusterConfigSpec: nil,
524-
nutanixWorkerNodeConfigSpecByMDName: nil,
525-
expectedChecks: 0,
526-
expectedControlPlaneCheckFieldIncluded: false,
502+
name: "client not initialized",
503+
nutanixClusterConfigSpec: nil,
504+
nutanixWorkerNodeConfigSpecByMDName: nil,
505+
v4client: nil,
506+
expectedChecks: 0,
507+
expectedControlPlaneCheckFieldIncluded: false,
508+
expectedWorkerNodeCheckFieldPatternExists: false,
509+
},
510+
{
511+
name: "no nutanix configuration",
512+
nutanixClusterConfigSpec: nil,
513+
nutanixWorkerNodeConfigSpecByMDName: nil,
514+
v4client: &mockv4client{},
515+
expectedChecks: 0,
516+
expectedControlPlaneCheckFieldIncluded: false,
527517
expectedWorkerNodeCheckFieldPatternExists: false,
528518
},
529519
{
@@ -541,6 +531,7 @@ func TestInitVMImageChecks(t *testing.T) {
541531
},
542532
},
543533
nutanixWorkerNodeConfigSpecByMDName: nil,
534+
v4client: &mockv4client{},
544535
expectedChecks: 1,
545536
expectedControlPlaneCheckFieldIncluded: true,
546537
expectedWorkerNodeCheckFieldPatternExists: false,
@@ -560,8 +551,9 @@ func TestInitVMImageChecks(t *testing.T) {
560551
},
561552
},
562553
},
563-
expectedChecks: 1,
564-
expectedControlPlaneCheckFieldIncluded: false,
554+
v4client: &mockv4client{},
555+
expectedChecks: 1,
556+
expectedControlPlaneCheckFieldIncluded: false,
565557
expectedWorkerNodeCheckFieldPatternExists: true,
566558
},
567559
{
@@ -600,8 +592,9 @@ func TestInitVMImageChecks(t *testing.T) {
600592
},
601593
},
602594
},
603-
expectedChecks: 3, // 1 control plane + 2 workers
604-
expectedControlPlaneCheckFieldIncluded: true,
595+
v4client: &mockv4client{},
596+
expectedChecks: 3, // 1 control plane + 2 workers
597+
expectedControlPlaneCheckFieldIncluded: true,
605598
expectedWorkerNodeCheckFieldPatternExists: true,
606599
},
607600
{
@@ -622,8 +615,9 @@ func TestInitVMImageChecks(t *testing.T) {
622615
},
623616
},
624617
},
625-
expectedChecks: 1, // only worker-2
626-
expectedControlPlaneCheckFieldIncluded: false,
618+
v4client: &mockv4client{},
619+
expectedChecks: 1, // only worker-2
620+
expectedControlPlaneCheckFieldIncluded: false,
627621
expectedWorkerNodeCheckFieldPatternExists: true,
628622
},
629623
{
@@ -644,6 +638,7 @@ func TestInitVMImageChecks(t *testing.T) {
644638
ControlPlane: nil, // null control plane
645639
},
646640
nutanixWorkerNodeConfigSpecByMDName: nil,
641+
v4client: &mockv4client{},
647642
expectedChecks: 0,
648643
expectedControlPlaneCheckFieldIncluded: false,
649644
expectedWorkerNodeCheckFieldPatternExists: false,
@@ -657,6 +652,7 @@ func TestInitVMImageChecks(t *testing.T) {
657652
log: logger,
658653
nutanixClusterConfigSpec: tc.nutanixClusterConfigSpec,
659654
nutanixWorkerNodeConfigSpecByMachineDeploymentName: tc.nutanixWorkerNodeConfigSpecByMDName,
655+
v4client: tc.v4client,
660656
}
661657

662658
// Trap the vmImageCheck calls to verify field paths

0 commit comments

Comments
 (0)