From 179f36f2085976565a416d71415a75a0578c9394 Mon Sep 17 00:00:00 2001 From: Mateusz Szostok Date: Mon, 2 Sep 2019 02:00:05 +0200 Subject: [PATCH] Add option to choose Lease lock if coordination group is available - Choose the Lease lock if lease.coordination.k8s.io is available otherwise, use ConfigMaps. - Add tests coverage for implemented logic using ginko & gomega - Remove outdated information about cert generation --- Gopkg.lock | 1 + pkg/leaderelection/doc.go | 5 +- pkg/leaderelection/leader_election.go | 33 ++++++-- .../leader_election_suite_test.go | 13 +++ pkg/leaderelection/leader_election_test.go | 79 +++++++++++++++++++ pkg/manager/manager.go | 4 +- pkg/webhook/server.go | 4 - 7 files changed, 127 insertions(+), 12 deletions(-) create mode 100644 pkg/leaderelection/leader_election_suite_test.go create mode 100644 pkg/leaderelection/leader_election_test.go diff --git a/Gopkg.lock b/Gopkg.lock index 0996047409..4fd45c9943 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -935,6 +935,7 @@ "k8s.io/api/apps/v1", "k8s.io/api/apps/v1beta1", "k8s.io/api/autoscaling/v1", + "k8s.io/api/coordination/v1", "k8s.io/api/core/v1", "k8s.io/api/extensions/v1beta1", "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1", diff --git a/pkg/leaderelection/doc.go b/pkg/leaderelection/doc.go index 24654faa3c..41f11291a7 100644 --- a/pkg/leaderelection/doc.go +++ b/pkg/leaderelection/doc.go @@ -19,6 +19,9 @@ Package leaderelection contains a constructors for a leader election resource lo This is used to ensure that multiple copies of a controller manager can be run with only one active set of controllers, for active-passive HA. -It uses built-in Kubernetes leader election APIs. +It uses built-in Kubernetes leader election APIs. The Lease lock type takes precedence +as edits to Leases are less common and fewer objects in the cluster watch "all Leases". +If the Lease API is not available, the ConfigMap resource lock is used. + */ package leaderelection diff --git a/pkg/leaderelection/leader_election.go b/pkg/leaderelection/leader_election.go index b3003ea5e3..97ef7b2196 100644 --- a/pkg/leaderelection/leader_election.go +++ b/pkg/leaderelection/leader_election.go @@ -21,6 +21,7 @@ import ( "io/ioutil" "os" + coordinationv1 "k8s.io/api/coordination/v1" "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -37,16 +38,17 @@ type Options struct { LeaderElection bool // LeaderElectionNamespace determines the namespace in which the leader - // election configmap will be created. + // election will be created. LeaderElectionNamespace string - // LeaderElectionID determines the name of the configmap that leader election + // LeaderElectionID determines the name of the resource lock that leader election // will use for holding the leader lock. LeaderElectionID string } -// NewResourceLock creates a new config map resource lock for use in a leader -// election loop +// NewResourceLock creates a new resource lock to use in a leader +// election loop. Choose the Lease lock if `lease.coordination.k8s.io` is available. +// Otherwise, the ConfigMap resource lock is used. func NewResourceLock(config *rest.Config, recorderProvider recorder.Provider, options Options) (resourcelock.Interface, error) { if !options.LeaderElection { return nil, nil @@ -79,8 +81,14 @@ func NewResourceLock(config *rest.Config, recorderProvider recorder.Provider, op return nil, err } + // Determine lock type + lockType, err := getDefaultLockType(client) + if err != nil { + return nil, err + } + // TODO(JoelSpeed): switch to leaderelection object in 1.12 - return resourcelock.New(resourcelock.ConfigMapsResourceLock, + return resourcelock.New(lockType, options.LeaderElectionNamespace, options.LeaderElectionID, client.CoreV1(), @@ -91,6 +99,21 @@ func NewResourceLock(config *rest.Config, recorderProvider recorder.Provider, op }) } +func getDefaultLockType(client *kubernetes.Clientset) (string, error) { + // check if new leader election api is available + supportedGroups, err := client.Discovery().ServerGroups() + if err != nil { + return "", fmt.Errorf("unable to retrieve supported server groups: %v", err) + } + for _, g := range supportedGroups.Groups { + if g.Name == coordinationv1.GroupName { + return resourcelock.LeasesResourceLock, nil + } + } + + return resourcelock.ConfigMapsResourceLock, nil +} + func getInClusterNamespace() (string, error) { // Check whether the namespace file exists. // If not, we are not running in cluster so can't guess the namespace. diff --git a/pkg/leaderelection/leader_election_suite_test.go b/pkg/leaderelection/leader_election_suite_test.go new file mode 100644 index 0000000000..b2f2a5b3ee --- /dev/null +++ b/pkg/leaderelection/leader_election_suite_test.go @@ -0,0 +1,13 @@ +package leaderelection + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestLeaderElection(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Leader Election Suite") +} diff --git a/pkg/leaderelection/leader_election_test.go b/pkg/leaderelection/leader_election_test.go new file mode 100644 index 0000000000..f351e00ea5 --- /dev/null +++ b/pkg/leaderelection/leader_election_test.go @@ -0,0 +1,79 @@ +package leaderelection + +import ( + "bytes" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + + tlog "github.com/go-logr/logr/testing" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + coordinationv1 "k8s.io/api/coordination/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + restclient "k8s.io/client-go/rest" + "k8s.io/client-go/tools/leaderelection/resourcelock" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/internal/recorder" +) + +var _ = Describe("Leader Election", func() { + It("should use the Lease lock because coordination group is available.", func() { + coordinationGroup := &v1.APIGroupList{ + Groups: []v1.APIGroup{ + {Name: coordinationv1.GroupName}, + }, + } + + clientConfig := &restclient.Config{ + Transport: interceptAPIGroupCall(coordinationGroup), + } + + rProvider, err := recorder.NewProvider(clientConfig, scheme.Scheme, tlog.NullLogger{}, record.NewBroadcaster()) + Expect(err).ToNot(HaveOccurred()) + + lock, err := NewResourceLock(clientConfig, rProvider, Options{LeaderElection: true, LeaderElectionNamespace: "test-ns"}) + Expect(err).ToNot(HaveOccurred()) + Expect(lock).To(BeAssignableToTypeOf(&resourcelock.LeaseLock{})) + }) + + It("should use the ConfigMap lock because coordination group is unavailable.", func() { + clientConfig := &restclient.Config{ + Transport: interceptAPIGroupCall(&v1.APIGroupList{ /* no coordination group */ }), + } + + rProvider, err := recorder.NewProvider(clientConfig, scheme.Scheme, tlog.NullLogger{}, record.NewBroadcaster()) + Expect(err).ToNot(HaveOccurred()) + + lock, err := NewResourceLock(clientConfig, rProvider, Options{LeaderElection: true, LeaderElectionNamespace: "test-ns"}) + Expect(err).ToNot(HaveOccurred()) + Expect(lock).To(BeAssignableToTypeOf(&resourcelock.ConfigMapLock{})) + }) +}) + +func interceptAPIGroupCall(returnApis *v1.APIGroupList) roundTripper { + return roundTripper(func(req *http.Request) (*http.Response, error) { + if req.Method == "GET" && (req.URL.Path == "/apis" || req.URL.Path == "/api") { + return encode(returnApis) + } + return nil, fmt.Errorf("unexpected request: %v %#v\n%#v", req.Method, req.URL, req) + }) +} +func encode(bodyStruct interface{}) (*http.Response, error) { + jsonBytes, err := json.Marshal(bodyStruct) + if err != nil { + return nil, err + } + return &http.Response{ + StatusCode: http.StatusOK, + Body: ioutil.NopCloser(bytes.NewReader(jsonBytes)), + }, nil +} + +type roundTripper func(*http.Request) (*http.Response, error) + +func (f roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 5be564507e..80aee9ce10 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -111,10 +111,10 @@ type Options struct { LeaderElection bool // LeaderElectionNamespace determines the namespace in which the leader - // election configmap will be created. + // election will be created. LeaderElectionNamespace string - // LeaderElectionID determines the name of the configmap that leader election + // LeaderElectionID determines the name of the resource lock that leader election // will use for holding the leader lock. LeaderElectionID string diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go index c7ac97abac..0313e412cc 100644 --- a/pkg/webhook/server.go +++ b/pkg/webhook/server.go @@ -53,10 +53,6 @@ type Server struct { Port int // CertDir is the directory that contains the server key and certificate. - // If using FSCertWriter in Provisioner, the server itself will provision the certificate and - // store it in this directory. - // If using SecretCertWriter in Provisioner, the server will provision the certificate in a secret, - // the user is responsible to mount the secret to the this location for the server to consume. CertDir string // WebhookMux is the multiplexer that handles different webhooks.