Skip to content

Commit

Permalink
Skips checking for readiness on CNI DEL (and instead warns)
Browse files Browse the repository at this point in the history
Because deletes should favor a successful path, the readiness check should be skipped for pod removals.

This can cause an issue where there's pods pending deletes and that might impact scheduling of a pod that may be necessary in order to set the readiness indicator.

Adds a new method  to check for readiness indicator alone in order to immediately log a warning.
  • Loading branch information
dougbtv committed Feb 21, 2024
1 parent 53a68c3 commit 1db7e61
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 2 deletions.
9 changes: 7 additions & 2 deletions pkg/multus/multus.go
Original file line number Diff line number Diff line change
Expand Up @@ -815,8 +815,13 @@ func CmdDel(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) er
}

if in.ReadinessIndicatorFile != "" {
if err := types.GetReadinessIndicatorFile(in.ReadinessIndicatorFile); err != nil {
return cmdErr(k8sArgs, "PollImmediate error waiting for ReadinessIndicatorFile (on del): %v", err)
readinessfileexists, err := types.ReadinessIndicatorExistsNow(in.ReadinessIndicatorFile)
if err != nil {
return cmdErr(k8sArgs, "error checking readinessindicatorfile @ %v: %v", in.ReadinessIndicatorFile, err)
} else {

Check warning on line 821 in pkg/multus/multus.go

View workflow job for this annotation

GitHub Actions / test (1.20.x, ubuntu-latest)

if block ends with a return statement, so drop this else and outdent its block

Check warning on line 821 in pkg/multus/multus.go

View workflow job for this annotation

GitHub Actions / test (1.21.x, ubuntu-latest)

if block ends with a return statement, so drop this else and outdent its block
if !readinessfileexists {
logging.Verbosef("warning: readinessindicatorfile @ %v does not exist", in.ReadinessIndicatorFile)
}
}
}

Expand Down
11 changes: 11 additions & 0 deletions pkg/types/conf.go
Original file line number Diff line number Diff line change
Expand Up @@ -618,3 +618,14 @@ func GetReadinessIndicatorFile(readinessIndicatorFile string) error {
return err == nil, nil
})
}

func ReadinessIndicatorExistsNow(readinessIndicatorFile string) (bool, error) {

Check warning on line 622 in pkg/types/conf.go

View workflow job for this annotation

GitHub Actions / test (1.20.x, ubuntu-latest)

exported function ReadinessIndicatorExistsNow should have comment or be unexported

Check warning on line 622 in pkg/types/conf.go

View workflow job for this annotation

GitHub Actions / test (1.21.x, ubuntu-latest)

exported function ReadinessIndicatorExistsNow should have comment or be unexported
_, err := os.Stat(readinessIndicatorFile)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
if err != nil {
if os.IsNotExist(err) {
return false, nil
}
return false, err
}
return true, nil
}

0 comments on commit 1db7e61

Please sign in to comment.