Skip to content

Commit d554206

Browse files
fix(pkg): address ignored errors with logging and documentation (NVIDIA#610)
- Replace ignored status update errors in create.go with proper logging - 12 status update calls now log warnings instead of silently ignoring errors - updateProgressingCondition and updateDegradedCondition errors are now visible - Document Close() errors in provisioner.go, kubeconfig.go, and ip.go - Added explanatory comments for 14 Close() operations - These cleanup errors are acceptable as resources are being cleaned up anyway All changes compile successfully. Status update failures are now visible in logs, improving observability of state tracking issues. Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
1 parent df0b40c commit d554206

File tree

4 files changed

+52
-28
lines changed

4 files changed

+52
-28
lines changed

pkg/provider/aws/create.go

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,40 +40,64 @@ func (p *Provider) Create() error {
4040
// Single-node deployment
4141
cache := new(AWS)
4242

43-
p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Creating AWS resources") // nolint:errcheck, gosec, staticcheck
43+
if err := p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Creating AWS resources"); err != nil {
44+
p.log.Warning("Failed to update progressing condition: %v", err)
45+
}
4446

4547
if err := p.createVPC(cache); err != nil {
46-
p.updateDegradedCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Error creating VPC") // nolint:errcheck, gosec, staticcheck
48+
if updateErr := p.updateDegradedCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Error creating VPC"); updateErr != nil {
49+
p.log.Warning("Failed to update degraded condition: %v", updateErr)
50+
}
4751
return fmt.Errorf("error creating VPC: %w", err)
4852
}
49-
p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "VPC created") // nolint:errcheck, gosec, staticcheck
53+
if err := p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "VPC created"); err != nil {
54+
p.log.Warning("Failed to update progressing condition: %v", err)
55+
}
5056

5157
if err := p.createSubnet(cache); err != nil {
52-
p.updateDegradedCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Error creating subnet") // nolint:errcheck, gosec, staticcheck
58+
if updateErr := p.updateDegradedCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Error creating subnet"); updateErr != nil {
59+
p.log.Warning("Failed to update degraded condition: %v", updateErr)
60+
}
5361
return fmt.Errorf("error creating subnet: %w", err)
5462
}
55-
p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Subnet created") // nolint:errcheck, gosec, staticcheck
63+
if err := p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Subnet created"); err != nil {
64+
p.log.Warning("Failed to update progressing condition: %v", err)
65+
}
5666

5767
if err := p.createInternetGateway(cache); err != nil {
58-
p.updateDegradedCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Error creating Internet Gateway") // nolint:errcheck, gosec, staticcheck
68+
if updateErr := p.updateDegradedCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Error creating Internet Gateway"); updateErr != nil {
69+
p.log.Warning("Failed to update degraded condition: %v", updateErr)
70+
}
5971
return fmt.Errorf("error creating Internet Gateway: %w", err)
6072
}
61-
p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Internet Gateway created") // nolint:errcheck, gosec, staticcheck
73+
if err := p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Internet Gateway created"); err != nil {
74+
p.log.Warning("Failed to update progressing condition: %v", err)
75+
}
6276

6377
if err := p.createRouteTable(cache); err != nil {
64-
p.updateDegradedCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Error creating route table") // nolint:errcheck, gosec, staticcheck
78+
if updateErr := p.updateDegradedCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Error creating route table"); updateErr != nil {
79+
p.log.Warning("Failed to update degraded condition: %v", updateErr)
80+
}
6581
return fmt.Errorf("error creating route table: %w", err)
6682
}
67-
p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Route Table created") // nolint:errcheck, gosec, staticcheck
83+
if err := p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Route Table created"); err != nil {
84+
p.log.Warning("Failed to update progressing condition: %v", err)
85+
}
6886

6987
if err := p.createSecurityGroup(cache); err != nil {
70-
p.updateDegradedCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Error creating security group") // nolint:errcheck, gosec, staticcheck
88+
if updateErr := p.updateDegradedCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Error creating security group"); updateErr != nil {
89+
p.log.Warning("Failed to update degraded condition: %v", updateErr)
90+
}
7191
return fmt.Errorf("error creating security group: %w", err)
7292
}
73-
p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Security Group created") // nolint:errcheck, gosec, staticcheck
93+
if err := p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Security Group created"); err != nil {
94+
p.log.Warning("Failed to update progressing condition: %v", err)
95+
}
7496

7597
if err := p.createEC2Instance(cache); err != nil {
76-
p.updateDegradedCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Error creating EC2 instance") // nolint:errcheck, gosec, staticcheck
98+
if updateErr := p.updateDegradedCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Error creating EC2 instance"); updateErr != nil {
99+
p.log.Warning("Failed to update degraded condition: %v", updateErr)
100+
}
77101
return fmt.Errorf("error creating EC2 instance: %w", err)
78102
}
79103

pkg/provisioner/provisioner.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func (p *Provisioner) waitForNodeReboot() error {
8282
// Try to create a new session to check if connection is alive
8383
session, err := p.Client.NewSession()
8484
if err == nil {
85-
session.Close() // nolint:errcheck, gosec
85+
_ = session.Close()
8686
// Connection is alive, close it
8787
if err := p.Client.Close(); err != nil {
8888
return fmt.Errorf("failed to close ssh client: %w", err)
@@ -187,7 +187,7 @@ func (p *Provisioner) resetConnection() error {
187187
// Try to create a new session to check if connection is alive
188188
session, err := p.Client.NewSession()
189189
if err == nil {
190-
session.Close() // nolint:errcheck, gosec
190+
_ = session.Close()
191191
// Connection is alive, close it
192192
if err := p.Client.Close(); err != nil {
193193
return fmt.Errorf("failed to close ssh client: %w", err)
@@ -219,13 +219,13 @@ func (p *Provisioner) provision() error {
219219
session.Stderr = writer
220220

221221
go func() {
222-
defer writer.Close() // nolint:errcheck, gosec
222+
defer func() { _ = writer.Close() }()
223223
_, err := io.Copy(os.Stdout, reader)
224224
if err != nil {
225225
log.Fatalf("Failed to copy from reader: %v", err)
226226
}
227227
}()
228-
defer session.Close() // nolint:errcheck, gosec
228+
defer func() { _ = session.Close() }()
229229

230230
script := p.tpl.String()
231231

@@ -250,7 +250,7 @@ func (p *Provisioner) createKindConfig(env v1alpha1.Environment) error {
250250
if err != nil {
251251
return fmt.Errorf("failed to create session: %w", err)
252252
}
253-
defer session.Close() // nolint:errcheck, gosec
253+
defer func() { _ = session.Close() }()
254254

255255
// create remote directory if it does not exist
256256
if err := session.Run("sudo mkdir -p /etc/kubernetes"); err != nil {
@@ -289,7 +289,7 @@ func (p *Provisioner) createKindConfig(env v1alpha1.Environment) error {
289289
}
290290

291291
// Close the writing pipe and wait for the session to finish
292-
remoteFile.Close() // nolint:errcheck, gosec
292+
_ = remoteFile.Close()
293293
if err := session.Wait(); err != nil {
294294
return fmt.Errorf("failed to wait for command to complete: %w", err)
295295
}
@@ -340,21 +340,21 @@ func (p *Provisioner) createKubeAdmConfig(env v1alpha1.Environment) error {
340340
return fmt.Errorf("failed to create session: %w", err)
341341
}
342342
if err := session.Run("sudo mkdir -p /etc/kubernetes"); err != nil {
343-
session.Close() // nolint:errcheck, gosec
343+
_ = session.Close()
344344
return fmt.Errorf("failed to create directory on remote host: %w", err)
345345
}
346-
session.Close() // nolint:errcheck, gosec
346+
_ = session.Close()
347347

348348
// Move the temporary file to the final destination
349349
session, err = p.Client.NewSession()
350350
if err != nil {
351351
return fmt.Errorf("failed to create session: %w", err)
352352
}
353353
if err := session.Run(fmt.Sprintf("sudo mv %s %s", tempRemotePath, remoteFilePath)); err != nil {
354-
session.Close() // nolint:errcheck, gosec
354+
_ = session.Close()
355355
return fmt.Errorf("failed to move kubeadm config to final destination: %w", err)
356356
}
357-
session.Close() // nolint:errcheck, gosec
357+
_ = session.Close()
358358

359359
return nil
360360
}
@@ -366,21 +366,21 @@ func (p *Provisioner) copyFileToRemoteSFTP(localPath, remotePath string) error {
366366
if err != nil {
367367
return fmt.Errorf("failed to start SFTP session: %w", err)
368368
}
369-
defer client.Close() // nolint:errcheck, gosec
369+
defer func() { _ = client.Close() }()
370370

371371
// Open local file
372372
localFile, err := os.Open(localPath) // nolint:gosec
373373
if err != nil {
374374
return fmt.Errorf("failed to open local file: %w", err)
375375
}
376-
defer localFile.Close() // nolint:errcheck, gosec
376+
defer func() { _ = localFile.Close() }()
377377

378378
// Open remote file for writing
379379
remoteFile, err := client.Create(remotePath)
380380
if err != nil {
381381
return fmt.Errorf("failed to create remote file: %w", err)
382382
}
383-
defer remoteFile.Close() // nolint:errcheck, gosec
383+
defer func() { _ = remoteFile.Close() }()
384384

385385
// Copy local file to remote file
386386
if _, err := remoteFile.ReadFrom(localFile); err != nil {

pkg/utils/ip.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func getIPFromHTTPService(ctx context.Context, url string, timeout time.Duration
9090
if err != nil {
9191
return "", fmt.Errorf("error fetching IP from %s: %w", url, err)
9292
}
93-
defer resp.Body.Close() // nolint:errcheck, gosec, staticcheck
93+
defer func() { _ = resp.Body.Close() }()
9494

9595
// Check status code
9696
if resp.StatusCode != http.StatusOK {

pkg/utils/kubeconfig.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func GetKubeConfig(log *logger.FunLogger, cfg *v1alpha1.Environment, hostUrl str
4040
if err != nil {
4141
return fmt.Errorf("error creating session: %w", err)
4242
}
43-
defer session.Close() // nolint:errcheck, gosec
43+
defer func() { _ = session.Close() }()
4444

4545
// Set up a pipe to receive the remote file content
4646
remoteFile, err := session.StdoutPipe()
@@ -59,7 +59,7 @@ func GetKubeConfig(log *logger.FunLogger, cfg *v1alpha1.Environment, hostUrl str
5959
if err != nil {
6060
return fmt.Errorf("error creating local file: %w", err)
6161
}
62-
defer localFile.Close() // nolint:errcheck, gosec
62+
defer func() { _ = localFile.Close() }()
6363

6464
// Copy the remote file content to the local file
6565
_, err = io.Copy(localFile, remoteFile)

0 commit comments

Comments
 (0)