From 0316dbf0d43f3f16fdc66e0dbfaa236b3b63e826 Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Sat, 7 Mar 2020 23:55:59 +0900 Subject: [PATCH 01/15] Add github package --- github/fake/fake.go | 64 ++++++++++++++++++ github/github.go | 148 ++++++++++++++++++++++++++++++++++++++++++ github/github_test.go | 130 +++++++++++++++++++++++++++++++++++++ 3 files changed, 342 insertions(+) create mode 100644 github/fake/fake.go create mode 100644 github/github.go create mode 100644 github/github_test.go diff --git a/github/fake/fake.go b/github/fake/fake.go new file mode 100644 index 0000000000..77b1fc0a76 --- /dev/null +++ b/github/fake/fake.go @@ -0,0 +1,64 @@ +package fake + +import ( + "fmt" + "net/http" + "net/http/httptest" + "time" +) + +const ( + RegistrationToken = "fake-registration-token" + + RunnersListBody = ` +[ + {"id": 1, "name": "test1", "os": "linux", "status": "online"}, + {"id": 2, "name": "test2", "os": "linux", "status": "offline"} +] +` +) + +func NewServer() *httptest.Server { + mux := http.NewServeMux() + + mux.HandleFunc("/repos/test/ok/actions/runners/registration-token", func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(http.StatusCreated) + expiresAt := time.Now().Add(time.Hour * 1) + fmt.Fprintf(w, fmt.Sprintf("{\"token\": \"%s\", \"expires_at\": \"%s\"}", RegistrationToken, expiresAt.Format(time.RFC3339))) + }) + mux.HandleFunc("/repos/test/invalid/actions/runners/registration-token", func(w http.ResponseWriter, req *http.Request) { + expiresAt := time.Now().Add(time.Hour * 1) + fmt.Fprintf(w, fmt.Sprintf("{\"token\": \"%s\", \"expires_at\": \"%s\"}", RegistrationToken, expiresAt.Format(time.RFC3339))) + }) + mux.HandleFunc("/repos/test/error/actions/runners/registration-token", func(w http.ResponseWriter, req *http.Request) { + http.Error(w, "", http.StatusBadRequest) + }) + + mux.HandleFunc("/repos/test/ok/actions/runners", func(w http.ResponseWriter, req *http.Request) { + fmt.Fprintf(w, RunnersListBody) + }) + mux.HandleFunc("/repos/test/invalid/actions/runners", func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(http.StatusNoContent) + }) + mux.HandleFunc("/repos/test/error/actions/runners", func(w http.ResponseWriter, req *http.Request) { + http.Error(w, "", http.StatusBadRequest) + }) + mux.HandleFunc("/repos/test/remove-invalid/actions/runners", func(w http.ResponseWriter, req *http.Request) { + fmt.Fprintf(w, RunnersListBody) + }) + mux.HandleFunc("/repos/test/remove-error/actions/runners", func(w http.ResponseWriter, req *http.Request) { + fmt.Fprintf(w, RunnersListBody) + }) + + mux.HandleFunc("/repos/test/ok/actions/runners/1", func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(http.StatusNoContent) + }) + mux.HandleFunc("/repos/test/remove-invalid/actions/runners/1", func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(http.StatusOK) + }) + mux.HandleFunc("/repos/test/remove-error/actions/runners/1", func(w http.ResponseWriter, req *http.Request) { + http.Error(w, "", http.StatusBadRequest) + }) + + return httptest.NewServer(mux) +} diff --git a/github/github.go b/github/github.go new file mode 100644 index 0000000000..517a06f2cc --- /dev/null +++ b/github/github.go @@ -0,0 +1,148 @@ +package github + +import ( + "context" + "fmt" + "sync" + "time" + + "github.com/google/go-github/v29/github" + "golang.org/x/oauth2" +) + +// Runner represents a runner in the response of GitHub API. +type Runner struct { + ID int `json:"id"` + Name string `json:"name"` + OS string `json:"os"` + Status string `json:"status"` +} + +// RegistrationToken represents a registration token in the response +// of GitHub API. +type RegistrationToken struct { + Token string `json:"token"` + ExpiresAt github.Timestamp `json:"expires_at"` +} + +// Client is a client for GitHub Actions API. +type Client struct { + github *github.Client + tokens map[string]*RegistrationToken + mu sync.Mutex +} + +// NewClient returns a client that uses the access token specified. +func NewClient(token string) *Client { + tc := oauth2.NewClient(context.Background(), oauth2.StaticTokenSource( + &oauth2.Token{AccessToken: token}, + )) + client := github.NewClient(tc) + + return &Client{ + github: client, + tokens: map[string]*RegistrationToken{}, + mu: sync.Mutex{}, + } +} + +// GetRegistrationToken returns a registration token tied with the name of repository and runner. +func (c *Client) GetRegistrationToken(ctx context.Context, repo, name string) (string, error) { + c.mu.Lock() + defer c.mu.Unlock() + + key := fmt.Sprintf("%s/%s", repo, name) + rt, ok := c.tokens[key] + if ok && rt.ExpiresAt.After(time.Now().Add(-10*time.Minute)) { + return rt.Token, nil + } + + req, err := c.github.NewRequest("POST", fmt.Sprintf("/repos/%s/actions/runners/registration-token", repo), nil) + if err != nil { + return "", err + } + + var regToken *RegistrationToken + res, err := c.github.Do(ctx, req, ®Token) + if err != nil { + return "", err + } + + if res.StatusCode != 201 { + return "", fmt.Errorf("unexpected status: %d", res.StatusCode) + } + + c.tokens[key] = regToken + + go func() { + c.cleanup() + }() + + return regToken.Token, nil +} + +// RemoveRunner removes a runner with specified name from repocitory. +func (c *Client) RemoveRunner(ctx context.Context, repo, name string) (bool, error) { + runners, err := c.ListRunners(ctx, repo) + if err != nil { + return false, err + } + + id := 0 + for _, runner := range runners { + if runner.Name == name { + id = runner.ID + break + } + } + + if id == 0 { + return false, nil + } + + req, err := c.github.NewRequest("DELETE", fmt.Sprintf("/repos/%s/actions/runners/%d", repo, id), nil) + if err != nil { + return false, err + } + + res, err := c.github.Do(ctx, req, nil) + if err != nil { + return false, err + } + + if res.StatusCode != 204 { + return false, fmt.Errorf("unexpected status: %d", res.StatusCode) + } + + return true, nil +} + +// ListRunners returns a list of runners of specified repository name. +func (c *Client) ListRunners(ctx context.Context, repo string) ([]Runner, error) { + runners := []Runner{} + + req, err := c.github.NewRequest("GET", fmt.Sprintf("/repos/%s/actions/runners", repo), nil) + if err != nil { + return runners, err + } + + res, err := c.github.Do(ctx, req, &runners) + if err != nil { + return runners, err + } + + if res.StatusCode != 200 { + return runners, fmt.Errorf("unexpected status: %d", res.StatusCode) + } + + return runners, nil +} + +// cleanup removes expired registration tokens. +func (c *Client) cleanup() { + for key, rt := range c.tokens { + if rt.ExpiresAt.Before(time.Now()) { + delete(c.tokens, key) + } + } +} diff --git a/github/github_test.go b/github/github_test.go new file mode 100644 index 0000000000..cc30761661 --- /dev/null +++ b/github/github_test.go @@ -0,0 +1,130 @@ +package github + +import ( + "context" + "net/http/httptest" + "net/url" + "testing" + "time" + + "github.com/google/go-github/v29/github" + "github.com/summerwind/actions-runner-controller/github/fake" +) + +var ( + server *httptest.Server + client *Client +) + +func newTestClient() *Client { + baseURL, err := url.Parse(server.URL + "/") + if err != nil { + panic(err) + } + + client := NewClient("token") + client.github.BaseURL = baseURL + + return client +} + +func TestMain(m *testing.M) { + server = fake.NewServer() + defer server.Close() + + client = newTestClient() + + m.Run() +} + +func TestGetRegistrationToken(t *testing.T) { + tests := []struct { + repo string + token string + err bool + }{ + {repo: "test/ok", token: fake.RegistrationToken, err: false}, + {repo: "test/ok", token: fake.RegistrationToken, err: false}, + {repo: "test/invalid", token: "", err: true}, + {repo: "test/error", token: "", err: true}, + } + + for _, tt := range tests { + token, err := client.GetRegistrationToken(context.Background(), tt.repo, "test") + if !tt.err && err != nil { + t.Errorf("unexpected error: %v", err) + } + if tt.token != token { + t.Errorf("unexpected token: %v", token) + } + } +} + +func TestListRunners(t *testing.T) { + tests := []struct { + repo string + length int + err bool + }{ + {repo: "test/ok", length: 2, err: false}, + {repo: "test/invalid", length: 0, err: true}, + {repo: "test/error", length: 0, err: true}, + } + + for _, tt := range tests { + runners, err := client.ListRunners(context.Background(), tt.repo) + if !tt.err && err != nil { + t.Errorf("unexpected error: %v", err) + } + if tt.length != len(runners) { + t.Errorf("unexpected runners list: %v", runners) + } + } +} + +func TestRemoveRunner(t *testing.T) { + tests := []struct { + repo string + name string + ok bool + err bool + }{ + {repo: "test/ok", name: "test1", ok: true, err: false}, + {repo: "test/ok", name: "test3", ok: false, err: false}, + {repo: "test/invalid", name: "test1", ok: false, err: true}, + {repo: "test/remove-invalid", name: "test1", ok: false, err: true}, + {repo: "test/remove-error", name: "test1", ok: false, err: true}, + } + + for _, tt := range tests { + ok, err := client.RemoveRunner(context.Background(), tt.repo, tt.name) + if !tt.err && err != nil { + t.Errorf("unexpected error: %v", err) + } + if !tt.ok && ok { + t.Errorf("unexpected result: %v", ok) + } + } +} + +func TestCleanup(t *testing.T) { + client := newTestClient() + client.tokens = map[string]*RegistrationToken{ + "active": &RegistrationToken{ + Token: "active-token", + ExpiresAt: github.Timestamp{time.Now().Add(time.Hour * 1)}, + }, + "expired": &RegistrationToken{ + Token: "expired-token", + ExpiresAt: github.Timestamp{time.Now().Add(-time.Hour * 1)}, + }, + } + + client.cleanup() + if _, ok := client.tokens["active"]; !ok { + t.Errorf("active token was accidentally removed") + } + if _, ok := client.tokens["expired"]; ok { + t.Errorf("expired token still exists") + } +} From bc483211a9b1962fa39052cfad6cc0c973d097a6 Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Sun, 8 Mar 2020 00:02:33 +0900 Subject: [PATCH 02/15] fixup! Add github package --- github/github_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github/github_test.go b/github/github_test.go index cc30761661..a257f6c875 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -112,11 +112,11 @@ func TestCleanup(t *testing.T) { client.tokens = map[string]*RegistrationToken{ "active": &RegistrationToken{ Token: "active-token", - ExpiresAt: github.Timestamp{time.Now().Add(time.Hour * 1)}, + ExpiresAt: github.Timestamp{Time: time.Now().Add(time.Hour * 1)}, }, "expired": &RegistrationToken{ Token: "expired-token", - ExpiresAt: github.Timestamp{time.Now().Add(-time.Hour * 1)}, + ExpiresAt: github.Timestamp{Time: time.Now().Add(-time.Hour * 1)}, }, } From 68ea9728da32f935209900462b48c2780ff85af9 Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Sun, 8 Mar 2020 19:15:06 +0900 Subject: [PATCH 03/15] Add mutation webhook implementation to inject registration token --- webhook/registration_token_injector.go | 78 +++++++++ webhook/registration_token_injector_test.go | 171 ++++++++++++++++++++ 2 files changed, 249 insertions(+) create mode 100644 webhook/registration_token_injector.go create mode 100644 webhook/registration_token_injector_test.go diff --git a/webhook/registration_token_injector.go b/webhook/registration_token_injector.go new file mode 100644 index 0000000000..d744313a58 --- /dev/null +++ b/webhook/registration_token_injector.go @@ -0,0 +1,78 @@ +package webhook + +import ( + "context" + "encoding/json" + "net/http" + + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/summerwind/actions-runner-controller/api/v1alpha1" + "github.com/summerwind/actions-runner-controller/github" +) + +// +kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,failurePolicy=ignore,groups="",resources=pods,verbs=create,versions=v1,name=runner-pod.webhook.actions.summerwind.dev + +type RegistrationTokenInjector struct { + GitHubClient *github.Client + Log logr.Logger + decoder *admission.Decoder +} + +func (t *RegistrationTokenInjector) Handle(ctx context.Context, req admission.Request) admission.Response { + pod := &corev1.Pod{} + err := t.decoder.Decode(req, pod) + if err != nil { + t.Log.Error(err, "Failed to decode request object") + return admission.Errored(http.StatusBadRequest, err) + } + + if pod.Annotations == nil { + pod.Annotations = map[string]string{} + } + + repo, ok := pod.Annotations[v1alpha1.KeyRunnerRepository] + if ok { + rt, err := t.GitHubClient.GetRegistrationToken(context.Background(), repo, pod.Name) + if err != nil { + t.Log.Error(err, "Failed to get new registration token") + return admission.Errored(http.StatusInternalServerError, err) + } + + for i, c := range pod.Spec.Containers { + if c.Name == v1alpha1.ContainerName { + env := []corev1.EnvVar{ + { + Name: v1alpha1.EnvRunnerRepository, + Value: repo, + }, + { + Name: v1alpha1.EnvRunnerToken, + Value: rt, + }, + } + pod.Spec.Containers[i].Env = append(pod.Spec.Containers[i].Env, env...) + } + } + + if pod.Spec.RestartPolicy != corev1.RestartPolicyOnFailure { + pod.Spec.RestartPolicy = corev1.RestartPolicyOnFailure + } + } + + buf, err := json.Marshal(pod) + if err != nil { + t.Log.Error(err, "Failed to encode new object") + return admission.Errored(http.StatusInternalServerError, err) + } + + res := admission.PatchResponseFromRaw(req.Object.Raw, buf) + return res +} + +func (t *RegistrationTokenInjector) InjectDecoder(d *admission.Decoder) error { + t.decoder = d + return nil +} diff --git a/webhook/registration_token_injector_test.go b/webhook/registration_token_injector_test.go new file mode 100644 index 0000000000..56a2403ba4 --- /dev/null +++ b/webhook/registration_token_injector_test.go @@ -0,0 +1,171 @@ +package webhook + +import ( + "context" + "encoding/json" + "net/http/httptest" + "net/url" + "testing" + + "github.com/go-logr/logr" + "github.com/summerwind/actions-runner-controller/api/v1alpha1" + "github.com/summerwind/actions-runner-controller/github" + "github.com/summerwind/actions-runner-controller/github/fake" + "gomodules.xyz/jsonpatch/v2" + admissionv1beta1 "k8s.io/api/admission/v1beta1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +var ( + scheme *runtime.Scheme + decoder *admission.Decoder + logger logr.Logger + ghClient *github.Client + server *httptest.Server +) + +func TestMain(m *testing.M) { + var err error + + scheme = runtime.NewScheme() + _ = clientgoscheme.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) + + decoder, err = admission.NewDecoder(scheme) + if err != nil { + panic(err) + } + + logger = log.NullLogger{} + + server = fake.NewServer() + defer server.Close() + + baseURL, err := url.Parse(server.URL + "/") + if err != nil { + panic(err) + } + + ghClient = github.NewClient("token") + ghClient.SetBaseURL(baseURL) + + m.Run() +} + +func TestHandle(t *testing.T) { + injector := RegistrationTokenInjector{ + Log: logger, + GitHubClient: ghClient, + decoder: decoder, + } + + runnerPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "runner", + Annotations: map[string]string{ + v1alpha1.KeyRunnerRepository: "test/ok", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + corev1.Container{ + Name: v1alpha1.ContainerName, + Image: "runner:latest", + }, + }, + RestartPolicy: corev1.RestartPolicy("Always"), + }, + } + + runnerErrorPod := runnerPod.DeepCopy() + runnerErrorPod.ObjectMeta.Annotations[v1alpha1.KeyRunnerRepository] = "test/error" + + normalPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "normal", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + corev1.Container{ + Name: "nginx", + Image: "nginx:latest", + }, + }, + RestartPolicy: corev1.RestartPolicy("Always"), + }, + } + + runnerPodPatches := []jsonpatch.JsonPatchOperation{ + { + Operation: "add", + Path: "/spec/containers/0/env", + Value: []map[string]string{ + { + "name": "RUNNER_REPO", + "value": "test/ok", + }, + { + "name": "RUNNER_TOKEN", + "value": "fake-registration-token", + }, + }, + }, + { + Operation: "replace", + Path: "/spec/restartPolicy", + Value: "OnFailure", + }, + } + + normalPodPatches := []jsonpatch.JsonPatchOperation{} + + tests := []struct { + pod *corev1.Pod + patches []jsonpatch.JsonPatchOperation + err bool + }{ + {runnerPod, runnerPodPatches, false}, + {normalPod, normalPodPatches, false}, + {runnerErrorPod, nil, true}, + } + + for _, tt := range tests { + buf, err := json.Marshal(tt.pod) + if err != nil { + t.Fatalf("failed to marshal pod resource: %s", err) + } + + req := admission.Request{ + AdmissionRequest: admissionv1beta1.AdmissionRequest{ + Object: runtime.RawExtension{ + Raw: buf, + }, + }, + } + + res := injector.Handle(context.Background(), req) + if tt.err { + if res.Allowed { + t.Errorf("unexpected response: %v", res) + } + } else { + ttBuf, err := json.Marshal(tt.patches) + if err != nil { + t.Fatalf("failed to marshal JSON patch: %s", err) + } + resBuf, err := json.Marshal(res.Patches) + if err != nil { + t.Fatalf("failed to marshal JSON patch: %s", err) + } + + if string(ttBuf) != string(resBuf) { + t.Errorf("unexpected patches - got: %v, want: %v", string(ttBuf), string(resBuf)) + } + } + } +} From 42763748092c28bda57a60a3b4e78066b87e2d6f Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Sun, 8 Mar 2020 19:16:27 +0900 Subject: [PATCH 04/15] fixup! Add github package --- github/github.go | 6 ++++++ github/github_test.go | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/github/github.go b/github/github.go index 517a06f2cc..9fa61ddf9b 100644 --- a/github/github.go +++ b/github/github.go @@ -3,6 +3,7 @@ package github import ( "context" "fmt" + "net/url" "sync" "time" @@ -46,6 +47,11 @@ func NewClient(token string) *Client { } } +// SetBaseURL sets the base URL for the GitHub API. +func (c *Client) SetBaseURL(baseURL *url.URL) { + c.github.BaseURL = baseURL +} + // GetRegistrationToken returns a registration token tied with the name of repository and runner. func (c *Client) GetRegistrationToken(ctx context.Context, repo, name string) (string, error) { c.mu.Lock() diff --git a/github/github_test.go b/github/github_test.go index a257f6c875..35011eac78 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -23,7 +23,7 @@ func newTestClient() *Client { } client := NewClient("token") - client.github.BaseURL = baseURL + client.SetBaseURL(baseURL) return client } From 83f8f593e024c0567b5dcc249f2ae9c99d1224a6 Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Sun, 8 Mar 2020 20:54:01 +0900 Subject: [PATCH 05/15] Use hostname as default runner name --- runner/entrypoint.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/runner/entrypoint.sh b/runner/entrypoint.sh index 5bef0a6357..2daa513fc4 100755 --- a/runner/entrypoint.sh +++ b/runner/entrypoint.sh @@ -1,8 +1,7 @@ #!/bin/bash if [ -z "${RUNNER_NAME}" ]; then - echo "RUNNER_NAME must be set" 1>&2 - exit 1 + RUNNER_NAME=$(hostname) fi if [ -z "${RUNNER_REPO}" ]; then From 35d82662392b2c18bde94b9d73e571a695e115e0 Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Sun, 8 Mar 2020 21:01:35 +0900 Subject: [PATCH 06/15] Add webhook handler --- main.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/main.go b/main.go index bce9413609..02a32f3cbe 100644 --- a/main.go +++ b/main.go @@ -17,20 +17,22 @@ limitations under the License. package main import ( - "context" "flag" "fmt" "os" - "github.com/google/go-github/v29/github" actionsv1alpha1 "github.com/summerwind/actions-runner-controller/api/v1alpha1" "github.com/summerwind/actions-runner-controller/controllers" - "golang.org/x/oauth2" + "github.com/summerwind/actions-runner-controller/github" + "github.com/summerwind/actions-runner-controller/webhook" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" // +kubebuilder:scaffold:imports ) @@ -46,6 +48,8 @@ var ( func init() { _ = clientgoscheme.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) + _ = appsv1.AddToScheme(scheme) _ = actionsv1alpha1.AddToScheme(scheme) // +kubebuilder:scaffold:scheme @@ -77,10 +81,7 @@ func main() { os.Exit(1) } - tc := oauth2.NewClient(context.Background(), oauth2.StaticTokenSource( - &oauth2.Token{AccessToken: ghToken}, - )) - ghClient := github.NewClient(tc) + ghClient := github.NewClient(ghToken) ctrl.SetLogger(zap.New(func(o *zap.Options) { o.Development = true @@ -134,6 +135,12 @@ func main() { } // +kubebuilder:scaffold:builder + injector := &webhook.RegistrationTokenInjector{ + GitHubClient: ghClient, + Log: ctrl.Log.WithName("webhook").WithName("RegistrationTokenInjector"), + } + mgr.GetWebhookServer().Register("/mutate-v1-pod", &admission.Webhook{Handler: injector}) + setupLog.Info("starting manager") if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { setupLog.Error(err, "problem running manager") From de2184130e279cc403c293684291d6f31e9f6a24 Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Sun, 8 Mar 2020 21:02:02 +0900 Subject: [PATCH 07/15] Update dependency --- go.mod | 9 +++------ go.sum | 11 +---------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index 592121d448..1aa95722da 100644 --- a/go.mod +++ b/go.mod @@ -3,18 +3,15 @@ module github.com/summerwind/actions-runner-controller go 1.13 require ( - github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751 // indirect - github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d // indirect - github.com/bradleyfalzon/ghinstallation v1.1.1 github.com/davecgh/go-spew v1.1.1 github.com/go-logr/logr v0.1.0 - github.com/google/go-github v17.0.0+incompatible + github.com/google/go-cmp v0.3.1 // indirect github.com/google/go-github/v29 v29.0.2 github.com/onsi/ginkgo v1.8.0 github.com/onsi/gomega v1.5.0 - github.com/prometheus/common v0.0.0-20181126121408-4724e9255275 + github.com/stretchr/testify v1.4.0 // indirect golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 - gopkg.in/alecthomas/kingpin.v2 v2.2.6 // indirect + gomodules.xyz/jsonpatch/v2 v2.0.1 k8s.io/api v0.0.0-20190918155943-95b840bb6a1f k8s.io/apimachinery v0.0.0-20190913080033-27d36303b655 k8s.io/client-go v0.0.0-20190918160344-1fbdaa4c8d90 diff --git a/go.sum b/go.sum index 3e1437f9da..055820e62d 100644 --- a/go.sum +++ b/go.sum @@ -18,18 +18,12 @@ github.com/PuerkitoBio/purell v1.1.0/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbt github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0= github.com/PuerkitoBio/urlesc v0.0.0-20160726150825-5bd2802263f2/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= -github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751 h1:JYp7IbQjafoB+tBA3gMyHYHrpOtNuDiK/uB5uXxq5wM= -github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= -github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d h1:UQZhZ2O0vMHr2cI+DC1Mbh0TJxzA3RcLoMsFw+aXw7E= -github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= github.com/asaskevich/govalidator v0.0.0-20180720115003-f9ffefc3facf/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY= github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973 h1:xJ4a3vCFaGF/jqvzLMYoU8P317H5OQ+Via4RmuPwCS0= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= github.com/blang/semver v3.5.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= -github.com/bradleyfalzon/ghinstallation v1.1.1 h1:pmBXkxgM1WeF8QYvDLT5kuQiHMcmf+X015GI0KM/E3I= -github.com/bradleyfalzon/ghinstallation v1.1.1/go.mod h1:vyCmHTciHx/uuyN82Zc3rXN3X2KTK8nUTCrTMwAhcug= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/coreos/bbolt v1.3.1-coreos.6/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= @@ -120,8 +114,6 @@ github.com/google/go-cmp v0.3.0 h1:crn/baboCvb5fXaQ0IJ1SGTsTVrWpDsCWC8EGETZijY= github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg= github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= -github.com/google/go-github v17.0.0+incompatible h1:N0LgJ1j65A7kfXrZnUDaYCs/Sf4rEjNlfyDHW9dolSY= -github.com/google/go-github v17.0.0+incompatible/go.mod h1:zLgOLi98H3fifZn+44m+umXrS52loVEgC2AApnigrVQ= github.com/google/go-github/v29 v29.0.2 h1:opYN6Wc7DOz7Ku3Oh4l7prmkOMwEcQxpFtxdU8N8Pts= github.com/google/go-github/v29 v29.0.2/go.mod h1:CHKiKKPHJ0REzfwc14QMklvtHwCveD0PxlMjLlzAM5E= github.com/google/go-querystring v1.0.0 h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk= @@ -235,6 +227,7 @@ github.com/stretchr/testify v0.0.0-20151208002404-e3a8ff8ce365/go.mod h1:a8OnRci github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= @@ -341,8 +334,6 @@ google.golang.org/genproto v0.0.0-20190418145605-e7d98fc518a7/go.mod h1:VzzqZJRn google.golang.org/genproto v0.0.0-20190502173448-54afdca5d873/go.mod h1:VzzqZJRnGkLBvHegQrXjBqPurQTc5/KpmUdxsrq26oE= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= -gopkg.in/alecthomas/kingpin.v2 v2.2.6 h1:jMFz6MfLP0/4fUyZle81rXUoxOBFi19VUFKVDOQfozc= -gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= From 6b9e4faf6959046e58b3472f9867b3e3914ae941 Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Mon, 9 Mar 2020 10:11:47 +0900 Subject: [PATCH 08/15] Add replicas field --- api/v1alpha1/runner_types.go | 61 ++++++++++++++++----------- api/v1alpha1/zz_generated.deepcopy.go | 11 ++++- 2 files changed, 46 insertions(+), 26 deletions(-) diff --git a/api/v1alpha1/runner_types.go b/api/v1alpha1/runner_types.go index 93cc30d0d6..4110ff32e3 100644 --- a/api/v1alpha1/runner_types.go +++ b/api/v1alpha1/runner_types.go @@ -21,6 +21,17 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + KeyRunnerName = "actions.summerwind.dev/runner-name" + KeyRunnerRepository = "actions.summerwind.dev/runner-repository" + + EnvRunnerName = "RUNNER_NAME" + EnvRunnerRepository = "RUNNER_REPO" + EnvRunnerToken = "RUNNER_TOKEN" + + ContainerName = "runner" +) + // RunnerSpec defines the desired state of Runner type RunnerSpec struct { // +kubebuilder:validation:MinLength=3 @@ -28,18 +39,34 @@ type RunnerSpec struct { Repository string `json:"repository"` // +optional - Image string `json:"image"` + Replicas *int32 `json:"replicas,omitempty"` + + // +optional + Image string `json:"image,omitempty"` // +optional - Env []corev1.EnvVar `json:"env"` + Env []corev1.EnvVar `json:"env,omitempty"` } // RunnerStatus defines the observed state of Runner type RunnerStatus struct { - Registration RunnerStatusRegistration `json:"registration"` - Phase string `json:"phase"` - Reason string `json:"reason"` - Message string `json:"message"` + // +optional + Registration *RunnerStatusRegistration `json:"registration,omitempty"` + Phase string `json:"phase"` + Reason string `json:"reason"` + Message string `json:"message"` + + // +optional + Replicas int32 `json:"replicas,omitempty"` + + // +optional + ReadyReplicas int32 `json:"readyReplicas,omitempty"` + + // +optional + CurrentReplicas int32 `json:"currentReplicas,omitempty"` + + // +optional + UpdatedReplicas int32 `json:"updatedReplicas,omitempty"` } type RunnerStatusRegistration struct { @@ -50,8 +77,9 @@ type RunnerStatusRegistration struct { // +kubebuilder:object:root=true // +kubebuilder:subresource:status -// +kubebuilder:printcolumn:JSONPath=".spec.repository",name=Repository,type=string -// +kubebuilder:printcolumn:JSONPath=".status.phase",name=Status,type=string +// +kubebuilder:printcolumn:JSONPath=".spec.replicas",name="Desired",type="integer",description="Desired Replicas" +// +kubebuilder:printcolumn:JSONPath=".status.replicas",name="Current",type="integer",description="Current Replicas" +// +kubebuilder:printcolumn:JSONPath=".status.readyReplicas",name="Ready",type="integer",description="Ready Replicas" // Runner is the Schema for the runners API type Runner struct { @@ -62,23 +90,6 @@ type Runner struct { Status RunnerStatus `json:"status,omitempty"` } -func (r Runner) IsRegisterable() bool { - if r.Status.Registration.Repository != r.Spec.Repository { - return false - } - - if r.Status.Registration.Token == "" { - return false - } - - now := metav1.Now() - if r.Status.Registration.ExpiresAt.Before(&now) { - return false - } - - return true -} - // +kubebuilder:object:root=true // RunnerList contains a list of Runner diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index b584e180c1..dfd3cfeb23 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -277,6 +277,11 @@ func (in *RunnerSetStatus) DeepCopy() *RunnerSetStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RunnerSpec) DeepCopyInto(out *RunnerSpec) { *out = *in + if in.Replicas != nil { + in, out := &in.Replicas, &out.Replicas + *out = new(int32) + **out = **in + } if in.Env != nil { in, out := &in.Env, &out.Env *out = make([]v1.EnvVar, len(*in)) @@ -299,7 +304,11 @@ func (in *RunnerSpec) DeepCopy() *RunnerSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RunnerStatus) DeepCopyInto(out *RunnerStatus) { *out = *in - in.Registration.DeepCopyInto(&out.Registration) + if in.Registration != nil { + in, out := &in.Registration, &out.Registration + *out = new(RunnerStatusRegistration) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RunnerStatus. From 2632000a3711d5999be123f50f26a16fca0c69cc Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Mon, 9 Mar 2020 10:12:07 +0900 Subject: [PATCH 09/15] Use StatefulSet instead of Pod to run multiple runners --- controllers/runner_controller.go | 414 +++++++++++++------------------ 1 file changed, 172 insertions(+), 242 deletions(-) diff --git a/controllers/runner_controller.go b/controllers/runner_controller.go index 8dd00add59..9bda62d95b 100644 --- a/controllers/runner_controller.go +++ b/controllers/runner_controller.go @@ -20,38 +20,23 @@ import ( "context" "fmt" "reflect" - "time" "github.com/go-logr/logr" - "github.com/google/go-github/v29/github" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/summerwind/actions-runner-controller/api/v1alpha1" + "github.com/summerwind/actions-runner-controller/github" ) -const ( - containerName = "runner" - finalizerName = "runner.actions.summerwind.dev" -) - -type GitHubRunner struct { - ID int `json:"id"` - Name string `json:"name"` - OS string `json:"os"` - Status string `json:"status"` -} - -type GitHubRegistrationToken struct { - Token string `json:"token"` - ExpiresAt string `json:"expires_at"` -} +const finalizerName = "runner.actions.summerwind.dev" // RunnerReconciler reconciles a Runner object type RunnerReconciler struct { @@ -66,7 +51,9 @@ type RunnerReconciler struct { // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runners,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runners/status,verbs=get;update;patch -// +kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch func (r *RunnerReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { ctx := context.Background() @@ -85,7 +72,7 @@ func (r *RunnerReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { newRunner.ObjectMeta.Finalizers = finalizers if err := r.Update(ctx, newRunner); err != nil { - log.Error(err, "Failed to update runner") + log.Error(err, "Failed to update Runner") return ctrl.Result{}, err } @@ -95,9 +82,9 @@ func (r *RunnerReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { finalizers, removed := removeFinalizer(runner.ObjectMeta.Finalizers) if removed { - ok, err := r.unregisterRunner(ctx, runner.Spec.Repository, runner.Name) + ok, err := r.GitHubClient.RemoveRunner(ctx, runner.Spec.Repository, runner.Name) if err != nil { - log.Error(err, "Failed to unregister runner") + log.Error(err, "Failed to remove runner from GitHub") return ctrl.Result{}, err } @@ -109,7 +96,7 @@ func (r *RunnerReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { newRunner.ObjectMeta.Finalizers = finalizers if err := r.Update(ctx, newRunner); err != nil { - log.Error(err, "Failed to update runner") + log.Error(err, "Failed to update Runner") return ctrl.Result{}, err } @@ -119,211 +106,105 @@ func (r *RunnerReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return ctrl.Result{}, nil } - if !runner.IsRegisterable() { - reg, err := r.newRegistration(ctx, runner.Spec.Repository) - if err != nil { - r.Recorder.Event(&runner, corev1.EventTypeWarning, "FailedUpdateRegistrationToken", "Updating registration token failed") - log.Error(err, "Failed to get new registration token") + var statefulSet appsv1.StatefulSet + if err := r.Get(ctx, req.NamespacedName, &statefulSet); err != nil { + if !errors.IsNotFound(err) { return ctrl.Result{}, err } - updated := runner.DeepCopy() - updated.Status.Registration = reg - - if err := r.Status().Update(ctx, updated); err != nil { - log.Error(err, "Failed to update runner status") + newStatefulSet, err := r.newStatefulSet(runner) + if err != nil { + log.Error(err, "Failed to generate StatefulSet from Runner") return ctrl.Result{}, err } - r.Recorder.Event(&runner, corev1.EventTypeNormal, "RegistrationTokenUpdated", "Successfully update registration token") - log.Info("Updated registration token", "repository", runner.Spec.Repository) - return ctrl.Result{}, nil - } - - var pod corev1.Pod - if err := r.Get(ctx, req.NamespacedName, &pod); err != nil { - if !errors.IsNotFound(err) { + if err := r.Create(ctx, &newStatefulSet); err != nil { + log.Error(err, "Failed to create StatefulSet") return ctrl.Result{}, err } - newPod, err := r.newPod(runner) - if err != nil { - log.Error(err, "Could not create pod") + r.Recorder.Event(&runner, corev1.EventTypeNormal, "StatefulSetCreated", fmt.Sprintf("Created StatefulSet '%s'", newStatefulSet.Name)) + log.Info("Created StatefulSet") + } else { + if !statefulSet.ObjectMeta.DeletionTimestamp.IsZero() { return ctrl.Result{}, err } - if err := r.Create(ctx, &newPod); err != nil { - log.Error(err, "Failed to create pod resource") - return ctrl.Result{}, err - } + if !isSpecInSync(runner.Spec, statefulSet.Spec) { + newStatefulSet := statefulSet.DeepCopy() + newStatefulSet.Spec.Template.Spec.Containers[0].Image = runner.Spec.Image + newStatefulSet.Spec.Template.Spec.Containers[0].Env = runner.Spec.Env + newStatefulSet.Spec.Replicas = runner.Spec.Replicas - r.Recorder.Event(&runner, corev1.EventTypeNormal, "PodCreated", fmt.Sprintf("Created pod '%s'", newPod.Name)) - log.Info("Created runner pod", "repository", runner.Spec.Repository) - } else { - if runner.Status.Phase != string(pod.Status.Phase) { - updated := runner.DeepCopy() - updated.Status.Phase = string(pod.Status.Phase) - updated.Status.Reason = pod.Status.Reason - updated.Status.Message = pod.Status.Message - - if err := r.Status().Update(ctx, updated); err != nil { - log.Error(err, "Failed to update runner status") + if err := r.Update(ctx, newStatefulSet); err != nil { + log.Error(err, "Failed to update StatefulSet") return ctrl.Result{}, err } - return ctrl.Result{}, nil - } + r.Recorder.Event(&runner, corev1.EventTypeNormal, "StatefulSetUpdated", fmt.Sprintf("Updated StatefulSet '%s'", newStatefulSet.Name)) + log.Info("Updated StatefulSet") - if !pod.ObjectMeta.DeletionTimestamp.IsZero() { - return ctrl.Result{}, err + return ctrl.Result{}, nil } - restart := false - - if pod.Status.Phase == corev1.PodRunning { - for _, status := range pod.Status.ContainerStatuses { - if status.Name != containerName { - continue - } + if !isStatusInSync(runner.Status, statefulSet.Status) { + newRunner := runner.DeepCopy() + newRunner.Status.Replicas = statefulSet.Status.Replicas + newRunner.Status.ReadyReplicas = statefulSet.Status.ReadyReplicas + newRunner.Status.CurrentReplicas = statefulSet.Status.CurrentReplicas + newRunner.Status.UpdatedReplicas = statefulSet.Status.UpdatedReplicas - if status.State.Terminated != nil && status.State.Terminated.ExitCode == 0 { - restart = true - } + if err := r.Status().Update(ctx, newRunner); err != nil { + log.Error(err, "Failed to update Runner status") + return ctrl.Result{}, err } - } - newPod, err := r.newPod(runner) - if err != nil { - log.Error(err, "Could not create pod") - return ctrl.Result{}, err - } + log.V(1).Info("Updated Runner status") - if pod.Spec.Containers[0].Image != newPod.Spec.Containers[0].Image { - restart = true - } - if !reflect.DeepEqual(pod.Spec.Containers[0].Env, newPod.Spec.Containers[0].Env) { - restart = true - } - if !restart { - return ctrl.Result{}, err + return ctrl.Result{}, nil } - if err := r.Delete(ctx, &pod); err != nil { - log.Error(err, "Failed to delete pod resource") + pods, err := r.getPods(statefulSet) + if err != nil { + log.Error(err, "Failed to get Pod list") return ctrl.Result{}, err } - r.Recorder.Event(&runner, corev1.EventTypeNormal, "PodDeleted", fmt.Sprintf("Deleted pod '%s'", newPod.Name)) - log.Info("Deleted runner pod", "repository", runner.Spec.Repository) - } + for _, pod := range pods { + restart := false - return ctrl.Result{}, nil -} - -func (r *RunnerReconciler) newRegistration(ctx context.Context, repo string) (v1alpha1.RunnerStatusRegistration, error) { - var reg v1alpha1.RunnerStatusRegistration - - rt, err := r.getRegistrationToken(ctx, repo) - if err != nil { - return reg, err - } - - expiresAt, err := time.Parse(time.RFC3339, rt.ExpiresAt) - if err != nil { - return reg, err - } - - reg.Repository = repo - reg.Token = rt.Token - reg.ExpiresAt = metav1.NewTime(expiresAt) - - return reg, err -} - -func (r *RunnerReconciler) getRegistrationToken(ctx context.Context, repo string) (GitHubRegistrationToken, error) { - var regToken GitHubRegistrationToken - - req, err := r.GitHubClient.NewRequest("POST", fmt.Sprintf("/repos/%s/actions/runners/registration-token", repo), nil) - if err != nil { - return regToken, err - } - - res, err := r.GitHubClient.Do(ctx, req, ®Token) - if err != nil { - return regToken, err - } + if !pod.ObjectMeta.DeletionTimestamp.IsZero() { + continue + } - if res.StatusCode != 201 { - return regToken, fmt.Errorf("unexpected status: %d", res.StatusCode) - } + if pod.Status.Phase == corev1.PodRunning { + for _, status := range pod.Status.ContainerStatuses { + if status.Name != v1alpha1.ContainerName { + continue + } - return regToken, nil -} + if status.State.Terminated != nil && status.State.Terminated.ExitCode == 0 { + restart = true + } + } + } -func (r *RunnerReconciler) unregisterRunner(ctx context.Context, repo, name string) (bool, error) { - runners, err := r.listRunners(ctx, repo) - if err != nil { - return false, err - } + if restart { + if err := r.Delete(ctx, &pod); err != nil { + log.Error(err, "Failed to delete pod") + return ctrl.Result{}, err + } - id := 0 - for _, runner := range runners { - if runner.Name == name { - id = runner.ID - break + r.Recorder.Event(&runner, corev1.EventTypeNormal, "PodDeleted", fmt.Sprintf("Deleted Pod '%s'", pod.Name)) + log.Info("Deleted Pod", "pod", pod.Name) + } } } - if id == 0 { - return false, nil - } - - if err := r.removeRunner(ctx, repo, id); err != nil { - return false, err - } - - return true, nil -} - -func (r *RunnerReconciler) listRunners(ctx context.Context, repo string) ([]GitHubRunner, error) { - runners := []GitHubRunner{} - - req, err := r.GitHubClient.NewRequest("GET", fmt.Sprintf("/repos/%s/actions/runners", repo), nil) - if err != nil { - return runners, err - } - - res, err := r.GitHubClient.Do(ctx, req, &runners) - if err != nil { - return runners, err - } - - if res.StatusCode != 200 { - return runners, fmt.Errorf("unexpected status: %d", res.StatusCode) - } - - return runners, nil -} - -func (r *RunnerReconciler) removeRunner(ctx context.Context, repo string, id int) error { - req, err := r.GitHubClient.NewRequest("DELETE", fmt.Sprintf("/repos/%s/actions/runners/%d", repo, id), nil) - if err != nil { - return err - } - - res, err := r.GitHubClient.Do(ctx, req, nil) - if err != nil { - return err - } - - if res.StatusCode != 204 { - return fmt.Errorf("unexpected status: %d", res.StatusCode) - } - - return nil + return ctrl.Result{}, nil } -func (r *RunnerReconciler) newPod(runner v1alpha1.Runner) (corev1.Pod, error) { +func (r *RunnerReconciler) newStatefulSet(runner v1alpha1.Runner) (appsv1.StatefulSet, error) { var ( privileged bool = true group int64 = 0 @@ -334,75 +215,93 @@ func (r *RunnerReconciler) newPod(runner v1alpha1.Runner) (corev1.Pod, error) { runnerImage = r.RunnerImage } - env := []corev1.EnvVar{ - { - Name: "RUNNER_NAME", - Value: runner.Name, - }, - { - Name: "RUNNER_REPO", - Value: runner.Spec.Repository, - }, - { - Name: "RUNNER_TOKEN", - Value: runner.Status.Registration.Token, - }, - } - env = append(env, runner.Spec.Env...) - - pod := corev1.Pod{ + statefulSet := appsv1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ Name: runner.Name, Namespace: runner.Namespace, }, - Spec: corev1.PodSpec{ - RestartPolicy: "OnFailure", - Containers: []corev1.Container{ - { - Name: containerName, - Image: runnerImage, - ImagePullPolicy: "Always", - Env: env, - VolumeMounts: []corev1.VolumeMount{ - { - Name: "docker", - MountPath: "/var/run", - }, + Spec: appsv1.StatefulSetSpec{ + ServiceName: runner.Name, + Replicas: runner.Spec.Replicas, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + v1alpha1.KeyRunnerName: runner.Name, + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + v1alpha1.KeyRunnerName: runner.Name, }, - SecurityContext: &corev1.SecurityContext{ - RunAsGroup: &group, + Annotations: map[string]string{ + v1alpha1.KeyRunnerRepository: runner.Spec.Repository, }, }, - { - Name: "docker", - Image: r.DockerImage, - VolumeMounts: []corev1.VolumeMount{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ { - Name: "docker", - MountPath: "/var/run", + Name: v1alpha1.ContainerName, + Image: runnerImage, + ImagePullPolicy: "Always", + Env: runner.Spec.Env, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "docker", + MountPath: "/var/run", + }, + }, + SecurityContext: &corev1.SecurityContext{ + RunAsGroup: &group, + }, + }, + { + Name: "docker", + Image: r.DockerImage, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "docker", + MountPath: "/var/run", + }, + }, + SecurityContext: &corev1.SecurityContext{ + Privileged: &privileged, + }, }, }, - SecurityContext: &corev1.SecurityContext{ - Privileged: &privileged, - }, - }, - }, - Volumes: []corev1.Volume{ - corev1.Volume{ - Name: "docker", - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + Volumes: []corev1.Volume{ + corev1.Volume{ + Name: "docker", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, }, }, }, }, } - if err := ctrl.SetControllerReference(&runner, &pod, r.Scheme); err != nil { - return pod, err + if err := ctrl.SetControllerReference(&runner, &statefulSet, r.Scheme); err != nil { + return statefulSet, err } - return pod, nil + return statefulSet, nil +} + +func (r *RunnerReconciler) getPods(statefulSet appsv1.StatefulSet) ([]corev1.Pod, error) { + podList := &corev1.PodList{} + opts := []client.ListOption{ + client.InNamespace(statefulSet.Namespace), + client.MatchingLabels(map[string]string{ + v1alpha1.KeyRunnerName: statefulSet.Name, + }), + } + + if err := r.List(context.Background(), podList, opts...); err != nil { + return nil, err + } + + return podList.Items, nil } func (r *RunnerReconciler) SetupWithManager(mgr ctrl.Manager) error { @@ -410,7 +309,7 @@ func (r *RunnerReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.Runner{}). - Owns(&corev1.Pod{}). + Owns(&appsv1.StatefulSet{}). Complete(r) } @@ -443,3 +342,34 @@ func removeFinalizer(finalizers []string) ([]string, bool) { return result, removed } + +func isSpecInSync(rs v1alpha1.RunnerSpec, ss appsv1.StatefulSetSpec) bool { + if rs.Image != ss.Template.Spec.Containers[0].Image { + return false + } + if !reflect.DeepEqual(rs.Env, ss.Template.Spec.Containers[0].Env) { + return false + } + if *rs.Replicas != *ss.Replicas { + return false + } + + return true +} + +func isStatusInSync(rs v1alpha1.RunnerStatus, ss appsv1.StatefulSetStatus) bool { + if rs.Replicas != ss.Replicas { + return false + } + if rs.ReadyReplicas != ss.ReadyReplicas { + return false + } + if rs.CurrentReplicas != ss.CurrentReplicas { + return false + } + if rs.UpdatedReplicas != ss.UpdatedReplicas { + return false + } + + return true +} From 7e44da83acd19e0a72a73617cdb20193480c28a8 Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Mon, 9 Mar 2020 10:13:09 +0900 Subject: [PATCH 10/15] Fix build failure of container image --- Dockerfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Dockerfile b/Dockerfile index 74eb9d7412..5acb1cbf96 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,6 +13,8 @@ RUN go mod download COPY main.go main.go COPY api/ api/ COPY controllers/ controllers/ +COPY webhook/ webhook/ +COPY github/ github/ # Build RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o manager main.go From f433156118a8716d1459084e827d8504cd9db4e8 Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Mon, 9 Mar 2020 10:18:48 +0900 Subject: [PATCH 11/15] Update CRD and RBAC manifests --- ...ions.summerwind.dev_runnerdeployments.yaml | 3 ++ .../bases/actions.summerwind.dev_runners.yaml | 34 +++++++++++++++---- .../actions.summerwind.dev_runnersets.yaml | 3 ++ config/rbac/role.yaml | 23 +++++++++++-- 4 files changed, 54 insertions(+), 9 deletions(-) diff --git a/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml b/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml index b5caf66fbe..0773c75256 100644 --- a/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml +++ b/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml @@ -163,6 +163,9 @@ spec: type: array image: type: string + replicas: + format: int32 + type: integer repository: minLength: 3 pattern: ^[^/]+/[^/]+$ diff --git a/config/crd/bases/actions.summerwind.dev_runners.yaml b/config/crd/bases/actions.summerwind.dev_runners.yaml index 96f732ebf5..2a2fa8be37 100644 --- a/config/crd/bases/actions.summerwind.dev_runners.yaml +++ b/config/crd/bases/actions.summerwind.dev_runners.yaml @@ -9,12 +9,18 @@ metadata: name: runners.actions.summerwind.dev spec: additionalPrinterColumns: - - JSONPath: .spec.repository - name: Repository - type: string - - JSONPath: .status.phase - name: Status - type: string + - JSONPath: .spec.replicas + description: Desired Replicas + name: Desired + type: integer + - JSONPath: .status.replicas + description: Current Replicas + name: Current + type: integer + - JSONPath: .status.readyReplicas + description: Ready Replicas + name: Ready + type: integer group: actions.summerwind.dev names: kind: Runner @@ -142,6 +148,9 @@ spec: type: array image: type: string + replicas: + format: int32 + type: integer repository: minLength: 3 pattern: ^[^/]+/[^/]+$ @@ -152,10 +161,16 @@ spec: status: description: RunnerStatus defines the observed state of Runner properties: + currentReplicas: + format: int32 + type: integer message: type: string phase: type: string + readyReplicas: + format: int32 + type: integer reason: type: string registration: @@ -172,11 +187,16 @@ spec: - repository - token type: object + replicas: + format: int32 + type: integer + updatedReplicas: + format: int32 + type: integer required: - message - phase - reason - - registration type: object type: object version: v1alpha1 diff --git a/config/crd/bases/actions.summerwind.dev_runnersets.yaml b/config/crd/bases/actions.summerwind.dev_runnersets.yaml index 5f5ddc7d7f..e3bb5df7c8 100644 --- a/config/crd/bases/actions.summerwind.dev_runnersets.yaml +++ b/config/crd/bases/actions.summerwind.dev_runnersets.yaml @@ -163,6 +163,9 @@ spec: type: array image: type: string + replicas: + format: int32 + type: integer repository: minLength: 3 pattern: ^[^/]+/[^/]+$ diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index eba62d65e6..e6435c00e3 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -6,6 +6,25 @@ metadata: creationTimestamp: null name: manager-role rules: +- apiGroups: + - "" + resources: + - events + verbs: + - create + - patch +- apiGroups: + - "" + resources: + - pods + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - actions.summerwind.dev resources: @@ -67,9 +86,9 @@ rules: - patch - update - apiGroups: - - "" + - apps resources: - - pods + - statefulsets verbs: - create - delete From 2120e9155b535d86f8351c4b6c169b5e504a6727 Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Mon, 9 Mar 2020 10:19:23 +0900 Subject: [PATCH 12/15] Enable mutation webhook --- config/default/kustomization.yaml | 60 ++++++++++---------- config/default/webhookcainjection_patch.yaml | 14 ++--- config/webhook/manifests.yaml | 25 ++++++++ 3 files changed, 62 insertions(+), 37 deletions(-) diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index 711219a787..d1be385a5e 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -17,9 +17,9 @@ bases: - ../rbac - ../manager # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in crd/kustomization.yaml -#- ../webhook +- ../webhook # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. -#- ../certmanager +- ../certmanager # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. #- ../prometheus @@ -36,39 +36,39 @@ patchesStrategicMerge: #- manager_prometheus_metrics_patch.yaml # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in crd/kustomization.yaml -#- manager_webhook_patch.yaml +- manager_webhook_patch.yaml # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. # Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks. # 'CERTMANAGER' needs to be enabled to use ca injection -#- webhookcainjection_patch.yaml +- webhookcainjection_patch.yaml # the following config is for teaching kustomize how to do var substitution vars: # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix. -#- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR -# objref: -# kind: Certificate -# group: cert-manager.io -# version: v1alpha2 -# name: serving-cert # this name should match the one in certificate.yaml -# fieldref: -# fieldpath: metadata.namespace -#- name: CERTIFICATE_NAME -# objref: -# kind: Certificate -# group: cert-manager.io -# version: v1alpha2 -# name: serving-cert # this name should match the one in certificate.yaml -#- name: SERVICE_NAMESPACE # namespace of the service -# objref: -# kind: Service -# version: v1 -# name: webhook-service -# fieldref: -# fieldpath: metadata.namespace -#- name: SERVICE_NAME -# objref: -# kind: Service -# version: v1 -# name: webhook-service +- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR + objref: + kind: Certificate + group: cert-manager.io + version: v1alpha2 + name: serving-cert # this name should match the one in certificate.yaml + fieldref: + fieldpath: metadata.namespace +- name: CERTIFICATE_NAME + objref: + kind: Certificate + group: cert-manager.io + version: v1alpha2 + name: serving-cert # this name should match the one in certificate.yaml +- name: SERVICE_NAMESPACE # namespace of the service + objref: + kind: Service + version: v1 + name: webhook-service + fieldref: + fieldpath: metadata.namespace +- name: SERVICE_NAME + objref: + kind: Service + version: v1 + name: webhook-service diff --git a/config/default/webhookcainjection_patch.yaml b/config/default/webhookcainjection_patch.yaml index 7e79bf9955..696b8cdef9 100644 --- a/config/default/webhookcainjection_patch.yaml +++ b/config/default/webhookcainjection_patch.yaml @@ -6,10 +6,10 @@ metadata: name: mutating-webhook-configuration annotations: cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) ---- -apiVersion: admissionregistration.k8s.io/v1beta1 -kind: ValidatingWebhookConfiguration -metadata: - name: validating-webhook-configuration - annotations: - cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) +#--- +#apiVersion: admissionregistration.k8s.io/v1beta1 +#kind: ValidatingWebhookConfiguration +#metadata: +# name: validating-webhook-configuration +# annotations: +# cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index e69de29bb2..f35f745ec5 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -0,0 +1,25 @@ + +--- +apiVersion: admissionregistration.k8s.io/v1beta1 +kind: MutatingWebhookConfiguration +metadata: + creationTimestamp: null + name: mutating-webhook-configuration +webhooks: +- clientConfig: + caBundle: Cg== + service: + name: webhook-service + namespace: system + path: /mutate-v1-pod + failurePolicy: Ignore + name: runner-pod.webhook.actions.summerwind.dev + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - CREATE + resources: + - pods From 00ff7384de4ba7388443539b1c95c057b9dd7fdb Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Mon, 9 Mar 2020 22:30:19 +0900 Subject: [PATCH 13/15] fixup! Add github package --- github/fake/fake.go | 16 +++++----------- github/github.go | 29 ++++++----------------------- github/github_test.go | 36 ++++++++++++++---------------------- 3 files changed, 25 insertions(+), 56 deletions(-) diff --git a/github/fake/fake.go b/github/fake/fake.go index 77b1fc0a76..9c5c140633 100644 --- a/github/fake/fake.go +++ b/github/fake/fake.go @@ -21,7 +21,7 @@ const ( func NewServer() *httptest.Server { mux := http.NewServeMux() - mux.HandleFunc("/repos/test/ok/actions/runners/registration-token", func(w http.ResponseWriter, req *http.Request) { + mux.HandleFunc("/repos/test/valid/actions/runners/registration-token", func(w http.ResponseWriter, req *http.Request) { w.WriteHeader(http.StatusCreated) expiresAt := time.Now().Add(time.Hour * 1) fmt.Fprintf(w, fmt.Sprintf("{\"token\": \"%s\", \"expires_at\": \"%s\"}", RegistrationToken, expiresAt.Format(time.RFC3339))) @@ -34,7 +34,7 @@ func NewServer() *httptest.Server { http.Error(w, "", http.StatusBadRequest) }) - mux.HandleFunc("/repos/test/ok/actions/runners", func(w http.ResponseWriter, req *http.Request) { + mux.HandleFunc("/repos/test/valid/actions/runners", func(w http.ResponseWriter, req *http.Request) { fmt.Fprintf(w, RunnersListBody) }) mux.HandleFunc("/repos/test/invalid/actions/runners", func(w http.ResponseWriter, req *http.Request) { @@ -43,20 +43,14 @@ func NewServer() *httptest.Server { mux.HandleFunc("/repos/test/error/actions/runners", func(w http.ResponseWriter, req *http.Request) { http.Error(w, "", http.StatusBadRequest) }) - mux.HandleFunc("/repos/test/remove-invalid/actions/runners", func(w http.ResponseWriter, req *http.Request) { - fmt.Fprintf(w, RunnersListBody) - }) - mux.HandleFunc("/repos/test/remove-error/actions/runners", func(w http.ResponseWriter, req *http.Request) { - fmt.Fprintf(w, RunnersListBody) - }) - mux.HandleFunc("/repos/test/ok/actions/runners/1", func(w http.ResponseWriter, req *http.Request) { + mux.HandleFunc("/repos/test/valid/actions/runners/1", func(w http.ResponseWriter, req *http.Request) { w.WriteHeader(http.StatusNoContent) }) - mux.HandleFunc("/repos/test/remove-invalid/actions/runners/1", func(w http.ResponseWriter, req *http.Request) { + mux.HandleFunc("/repos/test/invalid/actions/runners/1", func(w http.ResponseWriter, req *http.Request) { w.WriteHeader(http.StatusOK) }) - mux.HandleFunc("/repos/test/remove-error/actions/runners/1", func(w http.ResponseWriter, req *http.Request) { + mux.HandleFunc("/repos/test/error/actions/runners/1", func(w http.ResponseWriter, req *http.Request) { http.Error(w, "", http.StatusBadRequest) }) diff --git a/github/github.go b/github/github.go index 9fa61ddf9b..0722c724e8 100644 --- a/github/github.go +++ b/github/github.go @@ -88,39 +88,22 @@ func (c *Client) GetRegistrationToken(ctx context.Context, repo, name string) (s } // RemoveRunner removes a runner with specified name from repocitory. -func (c *Client) RemoveRunner(ctx context.Context, repo, name string) (bool, error) { - runners, err := c.ListRunners(ctx, repo) +func (c *Client) RemoveRunner(ctx context.Context, repo string, runnerID int) error { + req, err := c.github.NewRequest("DELETE", fmt.Sprintf("/repos/%s/actions/runners/%d", repo, runnerID), nil) if err != nil { - return false, err - } - - id := 0 - for _, runner := range runners { - if runner.Name == name { - id = runner.ID - break - } - } - - if id == 0 { - return false, nil - } - - req, err := c.github.NewRequest("DELETE", fmt.Sprintf("/repos/%s/actions/runners/%d", repo, id), nil) - if err != nil { - return false, err + return err } res, err := c.github.Do(ctx, req, nil) if err != nil { - return false, err + return err } if res.StatusCode != 204 { - return false, fmt.Errorf("unexpected status: %d", res.StatusCode) + return fmt.Errorf("unexpected status: %d", res.StatusCode) } - return true, nil + return nil } // ListRunners returns a list of runners of specified repository name. diff --git a/github/github_test.go b/github/github_test.go index 35011eac78..199c11407a 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -43,19 +43,18 @@ func TestGetRegistrationToken(t *testing.T) { token string err bool }{ - {repo: "test/ok", token: fake.RegistrationToken, err: false}, - {repo: "test/ok", token: fake.RegistrationToken, err: false}, + {repo: "test/valid", token: fake.RegistrationToken, err: false}, {repo: "test/invalid", token: "", err: true}, {repo: "test/error", token: "", err: true}, } - for _, tt := range tests { + for i, tt := range tests { token, err := client.GetRegistrationToken(context.Background(), tt.repo, "test") if !tt.err && err != nil { - t.Errorf("unexpected error: %v", err) + t.Errorf("[%d] unexpected error: %v", i, err) } if tt.token != token { - t.Errorf("unexpected token: %v", token) + t.Errorf("[%d] unexpected token: %v", i, token) } } } @@ -66,18 +65,18 @@ func TestListRunners(t *testing.T) { length int err bool }{ - {repo: "test/ok", length: 2, err: false}, + {repo: "test/valid", length: 2, err: false}, {repo: "test/invalid", length: 0, err: true}, {repo: "test/error", length: 0, err: true}, } - for _, tt := range tests { + for i, tt := range tests { runners, err := client.ListRunners(context.Background(), tt.repo) if !tt.err && err != nil { - t.Errorf("unexpected error: %v", err) + t.Errorf("[%d] unexpected error: %v", i, err) } if tt.length != len(runners) { - t.Errorf("unexpected runners list: %v", runners) + t.Errorf("[%d] unexpected runners list: %v", i, runners) } } } @@ -85,24 +84,17 @@ func TestListRunners(t *testing.T) { func TestRemoveRunner(t *testing.T) { tests := []struct { repo string - name string - ok bool err bool }{ - {repo: "test/ok", name: "test1", ok: true, err: false}, - {repo: "test/ok", name: "test3", ok: false, err: false}, - {repo: "test/invalid", name: "test1", ok: false, err: true}, - {repo: "test/remove-invalid", name: "test1", ok: false, err: true}, - {repo: "test/remove-error", name: "test1", ok: false, err: true}, + {repo: "test/valid", err: false}, + {repo: "test/invalid", err: true}, + {repo: "test/error", err: true}, } - for _, tt := range tests { - ok, err := client.RemoveRunner(context.Background(), tt.repo, tt.name) + for i, tt := range tests { + err := client.RemoveRunner(context.Background(), tt.repo, 1) if !tt.err && err != nil { - t.Errorf("unexpected error: %v", err) - } - if !tt.ok && ok { - t.Errorf("unexpected result: %v", ok) + t.Errorf("[%d] unexpected error: %v", i, err) } } } From c1b60229929f29a5cbdb18fbc88743eb893fcffa Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Mon, 9 Mar 2020 22:49:07 +0900 Subject: [PATCH 14/15] fixup! Add mutation webhook implementation to inject registration token --- webhook/registration_token_injector.go | 89 ++++++++--- webhook/registration_token_injector_test.go | 160 +++++++++++++------- 2 files changed, 171 insertions(+), 78 deletions(-) diff --git a/webhook/registration_token_injector.go b/webhook/registration_token_injector.go index d744313a58..dfe5c8969e 100644 --- a/webhook/registration_token_injector.go +++ b/webhook/registration_token_injector.go @@ -6,7 +6,12 @@ import ( "net/http" "github.com/go-logr/logr" + "gomodules.xyz/jsonpatch/v2" + admissionv1beta1 "k8s.io/api/admission/v1beta1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/summerwind/actions-runner-controller/api/v1alpha1" @@ -16,14 +21,15 @@ import ( // +kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,failurePolicy=ignore,groups="",resources=pods,verbs=create,versions=v1,name=runner-pod.webhook.actions.summerwind.dev type RegistrationTokenInjector struct { - GitHubClient *github.Client + client.Client Log logr.Logger + GitHubClient *github.Client decoder *admission.Decoder } func (t *RegistrationTokenInjector) Handle(ctx context.Context, req admission.Request) admission.Response { - pod := &corev1.Pod{} - err := t.decoder.Decode(req, pod) + var pod corev1.Pod + err := t.decoder.Decode(req, &pod) if err != nil { t.Log.Error(err, "Failed to decode request object") return admission.Errored(http.StatusBadRequest, err) @@ -33,33 +39,55 @@ func (t *RegistrationTokenInjector) Handle(ctx context.Context, req admission.Re pod.Annotations = map[string]string{} } - repo, ok := pod.Annotations[v1alpha1.KeyRunnerRepository] - if ok { - rt, err := t.GitHubClient.GetRegistrationToken(context.Background(), repo, pod.Name) - if err != nil { - t.Log.Error(err, "Failed to get new registration token") - return admission.Errored(http.StatusInternalServerError, err) + runnerName, okName := pod.Labels[v1alpha1.KeyRunnerName] + repo, okRepo := pod.Annotations[v1alpha1.KeyRunnerRepository] + if !okName || !okRepo { + return newEmptyResponse() + } + + nn := types.NamespacedName{ + Namespace: pod.Namespace, + Name: runnerName, + } + + var runner v1alpha1.Runner + if err := t.Get(ctx, nn, &runner); err != nil { + if errors.IsNotFound(err) { + return newEmptyResponse() } - for i, c := range pod.Spec.Containers { - if c.Name == v1alpha1.ContainerName { - env := []corev1.EnvVar{ - { - Name: v1alpha1.EnvRunnerRepository, - Value: repo, - }, - { - Name: v1alpha1.EnvRunnerToken, - Value: rt, - }, - } - pod.Spec.Containers[i].Env = append(pod.Spec.Containers[i].Env, env...) + t.Log.Error(err, "Failed to get Runner") + return admission.Errored(http.StatusInternalServerError, err) + } + + if runner.Spec.Repository != repo { + return newEmptyResponse() + } + + rt, err := t.GitHubClient.GetRegistrationToken(context.Background(), repo, pod.Name) + if err != nil { + t.Log.Error(err, "Failed to get new registration token") + return admission.Errored(http.StatusInternalServerError, err) + } + + for i, c := range pod.Spec.Containers { + if c.Name == v1alpha1.ContainerName { + env := []corev1.EnvVar{ + { + Name: v1alpha1.EnvRunnerRepository, + Value: repo, + }, + { + Name: v1alpha1.EnvRunnerToken, + Value: rt, + }, } + pod.Spec.Containers[i].Env = append(pod.Spec.Containers[i].Env, env...) } + } - if pod.Spec.RestartPolicy != corev1.RestartPolicyOnFailure { - pod.Spec.RestartPolicy = corev1.RestartPolicyOnFailure - } + if pod.Spec.RestartPolicy != corev1.RestartPolicyOnFailure { + pod.Spec.RestartPolicy = corev1.RestartPolicyOnFailure } buf, err := json.Marshal(pod) @@ -76,3 +104,14 @@ func (t *RegistrationTokenInjector) InjectDecoder(d *admission.Decoder) error { t.decoder = d return nil } + +func newEmptyResponse() admission.Response { + pt := admissionv1beta1.PatchTypeJSONPatch + return admission.Response{ + Patches: []jsonpatch.Operation{}, + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + PatchType: &pt, + }, + } +} diff --git a/webhook/registration_token_injector_test.go b/webhook/registration_token_injector_test.go index 56a2403ba4..a01e01aaef 100644 --- a/webhook/registration_token_injector_test.go +++ b/webhook/registration_token_injector_test.go @@ -5,6 +5,8 @@ import ( "encoding/json" "net/http/httptest" "net/url" + "path/filepath" + "sort" "testing" "github.com/go-logr/logr" @@ -16,13 +18,16 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) var ( - scheme *runtime.Scheme + sc *runtime.Scheme + kClient client.Client decoder *admission.Decoder logger logr.Logger ghClient *github.Client @@ -32,11 +37,25 @@ var ( func TestMain(m *testing.M) { var err error - scheme = runtime.NewScheme() - _ = clientgoscheme.AddToScheme(scheme) - _ = corev1.AddToScheme(scheme) + testEnv := &envtest.Environment{ + CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")}, + } + cfg, err := testEnv.Start() + if err != nil { + panic(err) + } + defer testEnv.Stop() + + sc = scheme.Scheme + _ = corev1.AddToScheme(sc) + _ = v1alpha1.AddToScheme(sc) - decoder, err = admission.NewDecoder(scheme) + kClient, err = client.New(cfg, client.Options{Scheme: sc}) + if err != nil { + panic(err) + } + + decoder, err = admission.NewDecoder(sc) if err != nil { panic(err) } @@ -58,56 +77,51 @@ func TestMain(m *testing.M) { } func TestHandle(t *testing.T) { - injector := RegistrationTokenInjector{ - Log: logger, - GitHubClient: ghClient, - decoder: decoder, - } - - runnerPod := &corev1.Pod{ + validRunner := v1alpha1.Runner{ ObjectMeta: metav1.ObjectMeta{ - Name: "runner", - Annotations: map[string]string{ - v1alpha1.KeyRunnerRepository: "test/ok", - }, + Namespace: "default", + Name: "test-valid", }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - corev1.Container{ - Name: v1alpha1.ContainerName, - Image: "runner:latest", - }, - }, - RestartPolicy: corev1.RestartPolicy("Always"), + Spec: v1alpha1.RunnerSpec{ + Repository: "test/valid", }, } - runnerErrorPod := runnerPod.DeepCopy() - runnerErrorPod.ObjectMeta.Annotations[v1alpha1.KeyRunnerRepository] = "test/error" + if err := kClient.Create(context.Background(), &validRunner); err != nil { + t.Fatalf("failed to create runner") + } + defer kClient.Delete(context.Background(), &validRunner) - normalPod := &corev1.Pod{ + errorRunner := v1alpha1.Runner{ ObjectMeta: metav1.ObjectMeta{ - Name: "normal", + Namespace: "default", + Name: "test-error", }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - corev1.Container{ - Name: "nginx", - Image: "nginx:latest", - }, - }, - RestartPolicy: corev1.RestartPolicy("Always"), + Spec: v1alpha1.RunnerSpec{ + Repository: "test/error", }, } - runnerPodPatches := []jsonpatch.JsonPatchOperation{ + if err := kClient.Create(context.Background(), &errorRunner); err != nil { + t.Fatalf("failed to create runner") + } + defer kClient.Delete(context.Background(), &errorRunner) + + injector := RegistrationTokenInjector{ + Client: kClient, + Log: logger, + GitHubClient: ghClient, + decoder: decoder, + } + + runnerPatches := []jsonpatch.JsonPatchOperation{ { Operation: "add", Path: "/spec/containers/0/env", Value: []map[string]string{ { "name": "RUNNER_REPO", - "value": "test/ok", + "value": "test/valid", }, { "name": "RUNNER_TOKEN", @@ -122,22 +136,50 @@ func TestHandle(t *testing.T) { }, } - normalPodPatches := []jsonpatch.JsonPatchOperation{} + emptyPatches := []jsonpatch.JsonPatchOperation{} tests := []struct { - pod *corev1.Pod - patches []jsonpatch.JsonPatchOperation - err bool + runnerName string + runnerRepo string + patches []jsonpatch.JsonPatchOperation + err bool }{ - {runnerPod, runnerPodPatches, false}, - {normalPod, normalPodPatches, false}, - {runnerErrorPod, nil, true}, + {"", "", emptyPatches, false}, + {"test-valid", "test/valid", runnerPatches, false}, + {"test-valid", "test/invalid", emptyPatches, false}, + {"test-notfound", "test/valid", emptyPatches, false}, + {"test-error", "test/error", nil, true}, } - for _, tt := range tests { - buf, err := json.Marshal(tt.pod) + for i, tt := range tests { + pod := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "test", + Labels: map[string]string{}, + Annotations: map[string]string{}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + corev1.Container{ + Name: v1alpha1.ContainerName, + Image: "runner:latest", + }, + }, + RestartPolicy: corev1.RestartPolicy("Always"), + }, + } + + if tt.runnerName != "" { + pod.ObjectMeta.Labels[v1alpha1.KeyRunnerName] = tt.runnerName + } + if tt.runnerRepo != "" { + pod.ObjectMeta.Annotations[v1alpha1.KeyRunnerRepository] = tt.runnerRepo + } + + buf, err := json.Marshal(&pod) if err != nil { - t.Fatalf("failed to marshal pod resource: %s", err) + t.Fatalf("[%d] failed to marshal pod resource: %s", i, err) } req := admission.Request{ @@ -151,20 +193,32 @@ func TestHandle(t *testing.T) { res := injector.Handle(context.Background(), req) if tt.err { if res.Allowed { - t.Errorf("unexpected response: %v", res) + t.Errorf("[%d] unexpected response: %v", i, res) } } else { + sort.Slice(res.Patches, func(i, j int) bool { + if res.Patches[i].Operation == res.Patches[j].Operation { + if len(res.Patches[i].Path) < len(res.Patches[i].Path) { + return true + } + } else if res.Patches[i].Operation == "add" { + return true + } + + return false + }) + ttBuf, err := json.Marshal(tt.patches) if err != nil { - t.Fatalf("failed to marshal JSON patch: %s", err) + t.Fatalf("[%d] failed to marshal JSON patch: %s", i, err) } resBuf, err := json.Marshal(res.Patches) if err != nil { - t.Fatalf("failed to marshal JSON patch: %s", err) + t.Fatalf("[%d] failed to marshal JSON patch: %s", i, err) } if string(ttBuf) != string(resBuf) { - t.Errorf("unexpected patches - got: %v, want: %v", string(ttBuf), string(resBuf)) + t.Errorf("[%d] unexpected patches - got: %v, want: %v", i, string(resBuf), string(ttBuf)) } } } From 4bad4ececd970b5ff77e7c332b0a85d5ebc209d2 Mon Sep 17 00:00:00 2001 From: Moto Ishizawa Date: Tue, 10 Mar 2020 19:48:03 +0900 Subject: [PATCH 15/15] fixup! fixup! Add mutation webhook implementation to inject registration token --- main.go | 1 + 1 file changed, 1 insertion(+) diff --git a/main.go b/main.go index 02a32f3cbe..c241c18906 100644 --- a/main.go +++ b/main.go @@ -136,6 +136,7 @@ func main() { // +kubebuilder:scaffold:builder injector := &webhook.RegistrationTokenInjector{ + Client: mgr.GetClient(), GitHubClient: ghClient, Log: ctrl.Log.WithName("webhook").WithName("RegistrationTokenInjector"), }