Skip to content

Commit

Permalink
Thick plugin should not wait for API readiness on CNI DEL
Browse files Browse the repository at this point in the history
This modifies the behavior on CNI DEL for the thick plugin to just check once for API readiness, as opposed to waiting.
  • Loading branch information
dougbtv committed May 14, 2024
1 parent 5a2597b commit 181f56f
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
9 changes: 9 additions & 0 deletions pkg/server/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,12 @@ func WaitUntilAPIReady(socketPath string) error {
return err == nil, nil
})
}

// CheckAPIReadyNow checks API readiness once
func CheckAPIReadyNow(socketPath string) error {
_, err := DoCNI(GetAPIEndpoint(MultusHealthAPIEndpoint), nil, SocketPath(socketPath))
if err != nil {
return fmt.Errorf("CheckAPIReadyNow: Daemon not reachable over socketfile: %v", err)
}
return nil
}
15 changes: 9 additions & 6 deletions pkg/server/api/shim.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,12 @@ type ShimNetConf struct {
LogToStderr bool `json:"logToStderr,omitempty"`
}

// Define a type for API readiness check functions

Check warning on line 43 in pkg/server/api/shim.go

View workflow job for this annotation

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

comment on exported type APIReadyCheckFunc should be of the form "APIReadyCheckFunc ..." (with optional leading article)

Check warning on line 43 in pkg/server/api/shim.go

View workflow job for this annotation

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

comment on exported type APIReadyCheckFunc should be of the form "APIReadyCheckFunc ..." (with optional leading article)
type APIReadyCheckFunc func(string) error

Check warning on line 44 in pkg/server/api/shim.go

View workflow job for this annotation

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

type name will be used as api.APIReadyCheckFunc by other packages, and that stutters; consider calling this ReadyCheckFunc

Check warning on line 44 in pkg/server/api/shim.go

View workflow job for this annotation

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

type name will be used as api.APIReadyCheckFunc by other packages, and that stutters; consider calling this ReadyCheckFunc

// CmdAdd implements the CNI spec ADD command handler
func CmdAdd(args *skel.CmdArgs) error {
response, cniVersion, err := postRequest(args)
response, cniVersion, err := postRequest(args, WaitUntilAPIReady)
if err != nil {
return logging.Errorf("CmdAdd (shim): %v", err)
}
Expand All @@ -53,7 +56,7 @@ func CmdAdd(args *skel.CmdArgs) error {

// CmdCheck implements the CNI spec CHECK command handler
func CmdCheck(args *skel.CmdArgs) error {
_, _, err := postRequest(args)
_, _, err := postRequest(args, WaitUntilAPIReady)
if err != nil {
return logging.Errorf("CmdCheck (shim): %v", err)
}
Expand All @@ -63,22 +66,22 @@ func CmdCheck(args *skel.CmdArgs) error {

// CmdDel implements the CNI spec DEL command handler
func CmdDel(args *skel.CmdArgs) error {
_, _, err := postRequest(args)
_, _, err := postRequest(args, CheckAPIReadyNow)
if err != nil {
// No error in DEL (as of CNI spec)
logging.Errorf("CmdDel (shim): %v", err)
}
return nil
}

func postRequest(args *skel.CmdArgs) (*Response, string, error) {
func postRequest(args *skel.CmdArgs, readinessCheck APIReadyCheckFunc) (*Response, string, error) {
multusShimConfig, err := shimConfig(args.StdinData)
if err != nil {
return nil, "", fmt.Errorf("invalid CNI configuration passed to multus-shim: %w", err)
}

// check API readiness
if err := WaitUntilAPIReady(multusShimConfig.MultusSocketDir); err != nil {
// Execute the readiness check as necessary (e.g. don't wait on CNI DEL)
if err := readinessCheck(multusShimConfig.MultusSocketDir); err != nil {
return nil, multusShimConfig.CNIVersion, err
}

Expand Down

0 comments on commit 181f56f

Please sign in to comment.