From 197877d1132647321eff3df4f8e81b65c18c7a70 Mon Sep 17 00:00:00 2001 From: Tomofumi Hayashi Date: Fri, 5 Jan 2024 14:27:31 +0900 Subject: [PATCH 1/2] Adds a wait to account for the possiblity of a not ready unix socket --- cmd/cert-approver/main.go | 10 +++++----- cmd/multus-daemon/main.go | 13 +------------ pkg/server/api/api.go | 16 ++++++++++++++++ pkg/server/api/shim.go | 13 ++++++++++++- 4 files changed, 34 insertions(+), 18 deletions(-) diff --git a/cmd/cert-approver/main.go b/cmd/cert-approver/main.go index 52068a7c0..91b948458 100644 --- a/cmd/cert-approver/main.go +++ b/cmd/cert-approver/main.go @@ -73,15 +73,15 @@ var ( // ControllerName provides controller name ControllerName = "csr-approver" // NamePrefix specifies which name in certification request should be target to approve - NamePrefix = "system:multus" + NamePrefix = "system:multus" // Organization specifies which org in certification request should be target to approve - Organization = []string{"system:multus"} + Organization = []string{"system:multus"} // Groups specifies which group in certification request should be target to approve - Groups = sets.New[string]("system:nodes", "system:multus", "system:authenticated") + Groups = sets.New[string]("system:nodes", "system:multus", "system:authenticated") // UserPrefixes specifies which name prefix in certification request should be target to approve - UserPrefixes = sets.New[string]("system:node", NamePrefix) + UserPrefixes = sets.New[string]("system:node", NamePrefix) // Usages specifies which usage in certification request should be target to approve - Usages = sets.New[certificatesv1.KeyUsage]( + Usages = sets.New[certificatesv1.KeyUsage]( certificatesv1.UsageDigitalSignature, certificatesv1.UsageClientAuth) ) diff --git a/cmd/multus-daemon/main.go b/cmd/multus-daemon/main.go index 26e862cb4..39d83671f 100644 --- a/cmd/multus-daemon/main.go +++ b/cmd/multus-daemon/main.go @@ -28,7 +28,6 @@ import ( "path/filepath" "sync" "syscall" - "time" utilwait "k8s.io/apimachinery/pkg/util/wait" @@ -113,7 +112,7 @@ func main() { // Wait until daemon ready logging.Verbosef("API readiness check") - if waitUntilAPIReady(daemonConf.SocketDir) != nil { + if api.WaitUntilAPIReady(daemonConf.SocketDir) != nil { logging.Panicf("failed to ready multus-daemon socket: %v", err) os.Exit(1) } @@ -140,16 +139,6 @@ func main() { logging.Verbosef("multus daemon is exited") } -func waitUntilAPIReady(socketPath string) error { - apiReadyPollDuration := 100 * time.Millisecond - apiReadyPollTimeout := 1000 * time.Millisecond - - return utilwait.PollImmediate(apiReadyPollDuration, apiReadyPollTimeout, func() (bool, error) { - _, err := api.DoCNI(api.GetAPIEndpoint(api.MultusHealthAPIEndpoint), nil, api.SocketPath(socketPath)) - return err == nil, nil - }) -} - func startMultusDaemon(ctx context.Context, daemonConfig *srv.ControllerNetConf, ignoreReadinessIndicator bool) error { if user, err := user.Current(); err != nil || user.Uid != "0" { return fmt.Errorf("failed to run multus-daemon with root: %v, now running in uid: %s", err, user.Uid) diff --git a/pkg/server/api/api.go b/pkg/server/api/api.go index 9f348545e..10e6239f6 100644 --- a/pkg/server/api/api.go +++ b/pkg/server/api/api.go @@ -22,9 +22,17 @@ import ( "net" "net/http" "strings" + "time" + + utilwait "k8s.io/apimachinery/pkg/util/wait" ) const ( + // APIReadyPollDuration specifies duration for API readiness check polling + APIReadyPollDuration = 100 * time.Millisecond + // APIReadyPollTimeout specifies timeout for API readiness check polling + APIReadyPollTimeout = 60000 * time.Millisecond + // MultusCNIAPIEndpoint is an endpoint for multus CNI request (for multus-shim) MultusCNIAPIEndpoint = "/cni" // MultusDelegateAPIEndpoint is an endpoint for multus delegate request (for hotplug) @@ -88,3 +96,11 @@ func CreateDelegateRequest(cniCommand, cniContainerID, cniNetNS, cniIFName, podN InterfaceAttributes: interfaceAttributes, } } + +// WaitUntilAPIReady checks API readiness +func WaitUntilAPIReady(socketPath string) error { + return utilwait.PollImmediate(APIReadyPollDuration, APIReadyPollTimeout, func() (bool, error) { + _, err := DoCNI(GetAPIEndpoint(MultusHealthAPIEndpoint), nil, SocketPath(socketPath)) + return err == nil, nil + }) +} diff --git a/pkg/server/api/shim.go b/pkg/server/api/shim.go index 18ea0778f..01018ddd4 100644 --- a/pkg/server/api/shim.go +++ b/pkg/server/api/shim.go @@ -24,6 +24,8 @@ import ( cnitypes "github.com/containernetworking/cni/pkg/types" "gopkg.in/k8snetworkplumbingwg/multus-cni.v4/pkg/logging" + + utilwait "k8s.io/apimachinery/pkg/util/wait" ) // ShimNetConf for the SHIM cni config file written in json @@ -77,12 +79,21 @@ func postRequest(args *skel.CmdArgs) (*Response, string, error) { return nil, "", fmt.Errorf("invalid CNI configuration passed to multus-shim: %w", err) } + // check API readiness + if err := WaitUntilAPIReady(multusShimConfig.MultusSocketDir); err != nil { + return nil, multusShimConfig.CNIVersion, err + } + cniRequest, err := newCNIRequest(args) if err != nil { return nil, multusShimConfig.CNIVersion, err } - body, err := DoCNI("http://dummy/cni", cniRequest, SocketPath(multusShimConfig.MultusSocketDir)) + var body []byte + err = utilwait.PollImmediate(APIReadyPollDuration, APIReadyPollTimeout, func() (bool, error) { + body, err = DoCNI("http://dummy/cni", cniRequest, SocketPath(multusShimConfig.MultusSocketDir)) + return err == nil, nil + }) if err != nil { return nil, multusShimConfig.CNIVersion, err } From 6e4f62f2f25ac2be3eb456673e20ba9a7b8f5863 Mon Sep 17 00:00:00 2001 From: Tomofumi Hayashi Date: Fri, 5 Jan 2024 14:32:09 +0900 Subject: [PATCH 2/2] disable revive's dot-imports in unit test files --- cmd/thin_entrypoint/main_test.go | 6 ++++-- pkg/server/config/config_suite_test.go | 2 ++ pkg/server/config/generator_test.go | 2 ++ pkg/server/config/manager_test.go | 2 ++ 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/cmd/thin_entrypoint/main_test.go b/cmd/thin_entrypoint/main_test.go index 84e6a7b3c..25609107b 100644 --- a/cmd/thin_entrypoint/main_test.go +++ b/cmd/thin_entrypoint/main_test.go @@ -1,12 +1,14 @@ package main +// disable dot-imports only for testing +//revive:disable:dot-imports import ( "fmt" "os" "testing" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" + . "github.com/onsi/ginkgo/v2" //nolint:golint + . "github.com/onsi/gomega" //nolint:golint ) func TestThinEntrypoint(t *testing.T) { diff --git a/pkg/server/config/config_suite_test.go b/pkg/server/config/config_suite_test.go index 8214cad75..8947650e2 100644 --- a/pkg/server/config/config_suite_test.go +++ b/pkg/server/config/config_suite_test.go @@ -14,6 +14,8 @@ package config +// disable dot-imports only for testing +//revive:disable:dot-imports import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" diff --git a/pkg/server/config/generator_test.go b/pkg/server/config/generator_test.go index a21a59c90..51889b16c 100644 --- a/pkg/server/config/generator_test.go +++ b/pkg/server/config/generator_test.go @@ -14,6 +14,8 @@ package config +// disable dot-imports only for testing +//revive:disable:dot-imports import ( "encoding/json" "fmt" diff --git a/pkg/server/config/manager_test.go b/pkg/server/config/manager_test.go index 8758fb7cc..e6d69cff4 100644 --- a/pkg/server/config/manager_test.go +++ b/pkg/server/config/manager_test.go @@ -14,6 +14,8 @@ package config +// disable dot-imports only for testing +//revive:disable:dot-imports import ( "context" "encoding/json"