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.
dougbtv committed Feb 22, 2024
1 parent 53a68c3 commit a1915e1
Showing 2 changed files with 34 additions and 5 deletions.
9 changes: 7 additions & 2 deletions pkg/multus/multus.go
Original file line number Diff line number Diff line change
@@ -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 on CNI DEL @ %v: %v", in.ReadinessIndicatorFile, err)
}

if !readinessfileexists {
logging.Verbosef("warning: readinessindicatorfile @ %v does not exist on CNI DEL", in.ReadinessIndicatorFile)
}
}

30 changes: 27 additions & 3 deletions pkg/types/conf.go
Original file line number Diff line number Diff line change
@@ -20,17 +20,17 @@ import (
"fmt"
"net"
"os"
"path/filepath"
"strings"
"time"

utilwait "k8s.io/apimachinery/pkg/util/wait"

"github.com/containernetworking/cni/libcni"
"github.com/containernetworking/cni/pkg/skel"
cni100 "github.com/containernetworking/cni/pkg/types/100"
"github.com/containernetworking/cni/pkg/version"
nadutils "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/utils"
"gopkg.in/k8snetworkplumbingwg/multus-cni.v4/pkg/logging"
utilwait "k8s.io/apimachinery/pkg/util/wait"
)

const (
@@ -610,11 +610,35 @@ func CheckSystemNamespaces(namespace string, systemNamespaces []string) bool {
}

// GetReadinessIndicatorFile waits for readinessIndicatorFile
func GetReadinessIndicatorFile(readinessIndicatorFile string) error {
func GetReadinessIndicatorFile(readinessIndicatorFileRaw string) error {
cleanpath := filepath.Clean(readinessIndicatorFileRaw)
readinessIndicatorFile, err := filepath.Abs(cleanpath)
if err != nil {
return fmt.Errorf("failed to get absolute path of readinessIndicatorFile: %v", err)
}

pollDuration := 1000 * time.Millisecond
pollTimeout := 45 * time.Second
return utilwait.PollImmediate(pollDuration, pollTimeout, func() (bool, error) {
_, err := os.Stat(readinessIndicatorFile)
return err == nil, nil
})
}

// ReadinessIndicatorExistsNow reports if the readiness indicator exists immediately.
func ReadinessIndicatorExistsNow(readinessIndicatorFileRaw string) (bool, error) {
cleanpath := filepath.Clean(readinessIndicatorFileRaw)
readinessIndicatorFile, err := filepath.Abs(cleanpath)
if err != nil {
return false, fmt.Errorf("failed to get absolute path of readinessIndicatorFile: %v", err)
}

_, 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 a1915e1

Please sign in to comment.