Skip to content

Commit

Permalink
Merge pull request #1279 from dougbtv/skip-api-ready-on-cni-del
Browse files Browse the repository at this point in the history
Thick plugin should not wait for API readiness on CNI DEL
  • Loading branch information
s1061123 committed May 15, 2024
2 parents 5a2597b + 181f56f commit 75c0245
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 75c0245

Please sign in to comment.