From 604e71772be506ea2e368245a61ce5d25de54ee0 Mon Sep 17 00:00:00 2001 From: Lucas Pinheiro Date: Fri, 19 Jan 2024 16:36:54 -0300 Subject: [PATCH 1/4] add unit tests --- .gitignore | 3 +- Dockerfile.dev | 5 + Makefile | 3 + docker-compose.yml | 16 ++++ go.mod | 2 + pkg/agenttypes/registry.go | 2 +- pkg/agenttypes/registry_test.go | 161 ++++++++++++++++++++++++++++++++ 7 files changed, 190 insertions(+), 2 deletions(-) create mode 100644 Dockerfile.dev create mode 100644 docker-compose.yml create mode 100644 pkg/agenttypes/registry_test.go diff --git a/.gitignore b/.gitignore index d163863..9da810b 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ -build/ \ No newline at end of file +build/ +junit-report.xml diff --git a/Dockerfile.dev b/Dockerfile.dev new file mode 100644 index 0000000..8c7790c --- /dev/null +++ b/Dockerfile.dev @@ -0,0 +1,5 @@ +FROM golang:1.21 + +RUN go install gotest.tools/gotestsum@latest + +WORKDIR /app diff --git a/Makefile b/Makefile index 648cb87..b2bc485 100644 --- a/Makefile +++ b/Makefile @@ -32,6 +32,9 @@ check.docker: check.prepare lint: revive -formatter friendly -config lint.toml ./... +test: + docker compose run --rm app gotestsum --format short-verbose --junitfile junit-report.xml --packages="./..." -- -p 1 + build: rm -rf build env GOOS=linux go build -o build/controller main.go diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 0000000..a333966 --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,16 @@ +version: '3.6' +services: + app: + build: + context: . + dockerfile: Dockerfile.dev + tty: true + environment: {} + command: "/build/controller" + container_name: 'agent-k8s-controller' + volumes: + - go-pkg-cache:/go + - .:/app +volumes: + go-pkg-cache: + driver: local diff --git a/go.mod b/go.mod index 898a991..072a201 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/renderedtext/agent-k8s-stack go 1.21 require ( + github.com/stretchr/testify v1.8.2 k8s.io/api v0.28.4 k8s.io/apimachinery v0.28.4 k8s.io/client-go v0.28.4 @@ -29,6 +30,7 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/spf13/pflag v1.0.5 // indirect golang.org/x/net v0.17.0 // indirect golang.org/x/oauth2 v0.8.0 // indirect diff --git a/pkg/agenttypes/registry.go b/pkg/agenttypes/registry.go index f567073..71bf0fd 100644 --- a/pkg/agenttypes/registry.go +++ b/pkg/agenttypes/registry.go @@ -54,7 +54,7 @@ func (r *Registry) OnUpdate(oldObj, newObj interface{}) { agentType, err := parseAgentType(newSecret) if err != nil { - klog.Errorf("Error when updating agent type: %v", err) + klog.Errorf("Error when parsing agent type: %v", err) return } diff --git a/pkg/agenttypes/registry_test.go b/pkg/agenttypes/registry_test.go new file mode 100644 index 0000000..bd9888b --- /dev/null +++ b/pkg/agenttypes/registry_test.go @@ -0,0 +1,161 @@ +package agenttypes + +import ( + "encoding/base64" + "fmt" + "math/rand" + "testing" + + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func Test__Registry(t *testing.T) { + r, err := NewRegistry() + require.NoError(t, err) + + t.Run("on add -> no startup parameters", func(t *testing.T) { + agentTypeName := randAgentTypeName() + secretName := randSecretName() + token := randomString(t) + require.Nil(t, r.Get(agentTypeName)) + + s := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName}, + Data: map[string][]byte{ + "agentTypeName": []byte(agentTypeName), + "registrationToken": []byte(token), + }, + } + + r.OnAdd(s, false) + agentType := r.Get(agentTypeName) + require.NotNil(t, agentType) + require.Equal(t, agentTypeName, agentType.AgentTypeName) + require.Equal(t, token, agentType.RegistrationToken) + require.Equal(t, secretName, agentType.SecretName) + require.Empty(t, agentType.AgentStartupParameters) + }) + + t.Run("on add -> with startup parameters", func(t *testing.T) { + agentTypeName := randAgentTypeName() + secretName := randSecretName() + token := randomString(t) + require.Nil(t, r.Get(agentTypeName)) + + s := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName}, + Data: map[string][]byte{ + "agentTypeName": []byte(agentTypeName), + "registrationToken": []byte(token), + "agentStartupParameters": []byte("--kubernetes-pod-spec my-spec"), + }, + } + + r.OnAdd(s, false) + agentType := r.Get(agentTypeName) + require.NotNil(t, agentType) + require.Equal(t, agentTypeName, agentType.AgentTypeName) + require.Equal(t, token, agentType.RegistrationToken) + require.Equal(t, []string{"--kubernetes-pod-spec", "my-spec"}, agentType.AgentStartupParameters) + }) + + t.Run("on update, same version -> nothing is updated", func(t *testing.T) { + agentTypeName := randAgentTypeName() + secretName := randSecretName() + oldToken := randomString(t) + require.Nil(t, r.Get(agentTypeName)) + + old := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName, ResourceVersion: "1"}, + Data: map[string][]byte{ + "agentTypeName": []byte(agentTypeName), + "registrationToken": []byte(oldToken), + }, + } + + // agent type is added + r.OnAdd(old, false) + + // update with same resource version + // does not update the agent type information. + new := old.DeepCopy() + new.Data["registrationToken"] = []byte(randomString(t)) + r.OnUpdate(old, new) + + agentType := r.Get(agentTypeName) + require.NotNil(t, agentType) + require.Equal(t, oldToken, agentType.RegistrationToken) + }) + + t.Run("on update -> agent type is updated", func(t *testing.T) { + agentTypeName := randAgentTypeName() + secretName := randSecretName() + oldToken := randomString(t) + require.Nil(t, r.Get(agentTypeName)) + + old := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName, ResourceVersion: "1"}, + Data: map[string][]byte{ + "agentTypeName": []byte(agentTypeName), + "registrationToken": []byte(oldToken), + }, + } + + // agent type is added + r.OnAdd(old, false) + + // update with same resource version + // does not update the agent type information. + new := old.DeepCopy() + newToken := randomString(t) + new.Data["registrationToken"] = []byte(newToken) + new.ObjectMeta.ResourceVersion = "2" + r.OnUpdate(old, new) + + agentType := r.Get(agentTypeName) + require.NotNil(t, agentType) + require.Equal(t, newToken, agentType.RegistrationToken) + }) + + t.Run("on delete -> agent type is deleted", func(t *testing.T) { + agentTypeName := randAgentTypeName() + secretName := randSecretName() + token := randomString(t) + require.Nil(t, r.Get(agentTypeName)) + + s := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName, ResourceVersion: "1"}, + Data: map[string][]byte{ + "agentTypeName": []byte(agentTypeName), + "registrationToken": []byte(token), + }, + } + + // agent type is added + r.OnAdd(s, false) + require.NotNil(t, r.Get(agentTypeName)) + + // agent type is deleted + r.OnDelete(s) + require.Nil(t, r.Get(agentTypeName)) + }) +} + +func randAgentTypeName() string { + return fmt.Sprintf("s1-test-%d", rand.Int()) +} + +func randSecretName() string { + return fmt.Sprintf("secret-%d", rand.Int()) +} + +func randomString(t *testing.T) string { + buffer := make([]byte, 15) + + // #nosec + _, err := rand.Read(buffer) + require.NoError(t, err) + return base64.URLEncoding.EncodeToString(buffer) +} From 1341b220f9d2ff2d5d838944d3fd3686930616b5 Mon Sep 17 00:00:00 2001 From: Lucas Pinheiro Date: Fri, 19 Jan 2024 17:02:15 -0300 Subject: [PATCH 2/4] run tests in CI --- .semaphore/semaphore.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.semaphore/semaphore.yml b/.semaphore/semaphore.yml index a49fb47..a1f840e 100644 --- a/.semaphore/semaphore.yml +++ b/.semaphore/semaphore.yml @@ -5,7 +5,7 @@ agent: type: e2-standard-2 os_image: ubuntu2004 blocks: - - name: 'Lint' + - name: Tests dependencies: [] task: jobs: @@ -15,6 +15,10 @@ blocks: - checkout - go install github.com/mgechev/revive@latest - make lint + - name: Unit tests + commands: + - checkout + - make test - name: "Security checks" dependencies: [] task: From 0d5bad1270045f881731b2ad9187200e44bfb997 Mon Sep 17 00:00:00 2001 From: Lucas Pinheiro Date: Fri, 19 Jan 2024 17:21:47 -0300 Subject: [PATCH 3/4] more tests --- .semaphore/semaphore.yml | 4 + go.mod | 2 + go.sum | 4 + pkg/controller/job_scheduler.go | 5 ++ pkg/controller/job_scheduler_test.go | 129 +++++++++++++++++++++++++++ 5 files changed, 144 insertions(+) create mode 100644 pkg/controller/job_scheduler_test.go diff --git a/.semaphore/semaphore.yml b/.semaphore/semaphore.yml index a1f840e..6f1d00d 100644 --- a/.semaphore/semaphore.yml +++ b/.semaphore/semaphore.yml @@ -19,6 +19,10 @@ blocks: commands: - checkout - make test + epilogue: + always: + commands: + - test-results publish junit-report.xml - name: "Security checks" dependencies: [] task: diff --git a/go.mod b/go.mod index 072a201..79e17ae 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/emicklei/go-restful/v3 v3.9.0 // indirect + github.com/evanphx/json-patch v4.12.0+incompatible // indirect github.com/go-logr/logr v1.4.1 // indirect github.com/go-openapi/jsonpointer v0.19.6 // indirect github.com/go-openapi/jsonreference v0.20.2 // indirect @@ -30,6 +31,7 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect + github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/spf13/pflag v1.0.5 // indirect golang.org/x/net v0.17.0 // indirect diff --git a/go.sum b/go.sum index 8cd49fd..fdd5f40 100644 --- a/go.sum +++ b/go.sum @@ -4,6 +4,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/emicklei/go-restful/v3 v3.9.0 h1:XwGDlfxEnQZzuopoqxwSEllNcCOM9DhhFyhFIIGKwxE= github.com/emicklei/go-restful/v3 v3.9.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= +github.com/evanphx/json-patch v4.12.0+incompatible h1:4onqiflcdA9EOZ4RxV643DvftH5pOlLGNtQ5lPWQu84= +github.com/evanphx/json-patch v4.12.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ= github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-openapi/jsonpointer v0.19.6 h1:eCs3fxoIi3Wh6vtgmLTOjdhSpiqphQ+DaPn38N2ZdrE= @@ -60,6 +62,8 @@ github.com/onsi/ginkgo/v2 v2.9.4 h1:xR7vG4IXt5RWx6FfIjyAtsoMAtnc3C/rFXBBd2AjZwE= github.com/onsi/ginkgo/v2 v2.9.4/go.mod h1:gCQYp2Q+kSoIj7ykSVb9nskRSsR6PUj4AiLywzIhbKM= github.com/onsi/gomega v1.27.6 h1:ENqfyGeS5AX/rlXDd/ETokDz93u0YufY1Pgxuy/PvWE= github.com/onsi/gomega v1.27.6/go.mod h1:PIQNjfQwkP3aQAH7lf7j87O/5FiNr+ZR8+ipb+qQlhg= +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= diff --git a/pkg/controller/job_scheduler.go b/pkg/controller/job_scheduler.go index 47a2be8..3cfd49a 100644 --- a/pkg/controller/job_scheduler.go +++ b/pkg/controller/job_scheduler.go @@ -280,6 +280,11 @@ func (s *JobScheduler) OnDelete(obj interface{}) { Info("Job deleted") } +func (s *JobScheduler) JobExists(jobID string) bool { + _, ok := s.current[jobID] + return ok +} + func getFailedMessage(job *batchv1.Job) string { for _, cond := range job.Status.Conditions { if cond.Type == batchv1.JobFailed { diff --git a/pkg/controller/job_scheduler_test.go b/pkg/controller/job_scheduler_test.go new file mode 100644 index 0000000..95115c2 --- /dev/null +++ b/pkg/controller/job_scheduler_test.go @@ -0,0 +1,129 @@ +package controller + +import ( + "context" + "fmt" + "math/rand" + "testing" + + "github.com/renderedtext/agent-k8s-stack/pkg/agenttypes" + "github.com/renderedtext/agent-k8s-stack/pkg/config" + "github.com/renderedtext/agent-k8s-stack/pkg/semaphore" + "github.com/stretchr/testify/require" + batchv1 "k8s.io/api/batch/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/fake" +) + +func Test__JobScheduler(t *testing.T) { + agentType := agenttypes.AgentType{ + AgentTypeName: "s1-test", + RegistrationToken: "very-sensitive-token", + AgentStartupParameters: []string{}, + } + + maxParallelJobs := 5 + clientset := newFakeClientset([]runtime.Object{}) + scheduler := NewJobScheduler(clientset, &config.Config{ + Namespace: "default", + AgentImage: "semaphoreci/agent:latest", + AgentStartupParameters: []string{}, + Labels: []string{}, + MaxParallelJobs: maxParallelJobs, + }) + + t.Run("job is loaded on startup", func(t *testing.T) { + clear(scheduler.current) + defer clear(scheduler.current) + + jobID := randJobID() + require.False(t, scheduler.JobExists(jobID)) + + j := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + config.JobIDLabel: jobID, + config.AgentTypeLabel: agentType.AgentTypeName, + }, + }, + } + + scheduler.OnAdd(j, false) + require.True(t, scheduler.JobExists(jobID)) + }) + + t.Run("job is created", func(t *testing.T) { + clear(scheduler.current) + defer clear(scheduler.current) + + jobID := randJobID() + jobDoesNotExist(t, scheduler, clientset, jobID) + req := semaphore.JobRequest{JobID: jobID, MachineType: agentType.AgentTypeName} + err := scheduler.Create(context.Background(), req, &agentType) + require.NoError(t, err) + jobExists(t, scheduler, clientset, jobID) + + // Job creation is idempotent + require.NoError(t, scheduler.Create(context.Background(), req, &agentType)) + }) + + t.Run("job is not created if limit was reached", func(t *testing.T) { + clear(scheduler.current) + defer clear(scheduler.current) + + // create jobs up to max + for i := 0; i < maxParallelJobs; i++ { + jobID := randJobID() + req := semaphore.JobRequest{JobID: jobID, MachineType: agentType.AgentTypeName} + require.NoError(t, scheduler.Create(context.Background(), req, &agentType)) + _ = jobExists(t, scheduler, clientset, jobID) + } + + // creating a job returns an error now + jobID := randJobID() + req := semaphore.JobRequest{JobID: jobID, MachineType: agentType.AgentTypeName} + err := scheduler.Create(context.Background(), req, &agentType) + require.ErrorIs(t, err, ErrParallelJobsLimitReached) + jobDoesNotExist(t, scheduler, clientset, jobID) + }) + + t.Run("job is deleted", func(t *testing.T) { + clear(scheduler.current) + defer clear(scheduler.current) + + // job is created + jobID := randJobID() + req := semaphore.JobRequest{JobID: jobID, MachineType: agentType.AgentTypeName} + require.NoError(t, scheduler.Create(context.Background(), req, &agentType)) + j := jobExists(t, scheduler, clientset, jobID) + + scheduler.OnDelete(j) + require.False(t, scheduler.JobExists(jobID)) + }) +} + +func jobExists(t *testing.T, scheduler *JobScheduler, clientset kubernetes.Interface, jobID string) *batchv1.Job { + j, err := clientset.BatchV1().Jobs("default").Get(context.Background(), scheduler.jobName(jobID), metav1.GetOptions{}) + require.NoError(t, err) + require.NotNil(t, j) + require.True(t, scheduler.JobExists(jobID)) + return j +} + +func jobDoesNotExist(t *testing.T, scheduler *JobScheduler, clientset kubernetes.Interface, jobID string) { + _, err := clientset.BatchV1().Jobs("default").Get(context.Background(), scheduler.jobName(jobID), metav1.GetOptions{}) + require.Error(t, err) + require.True(t, errors.IsNotFound(err)) + require.False(t, scheduler.JobExists(jobID)) +} + +func newFakeClientset(objects []runtime.Object) kubernetes.Interface { + return fake.NewSimpleClientset(objects...) +} + +func randJobID() string { + return fmt.Sprintf("job-%d", rand.Int()) +} From da663c3667f41e2cd99cd869ddf699d2e4ef9bfe Mon Sep 17 00:00:00 2001 From: Lucas Pinheiro Date: Fri, 19 Jan 2024 17:25:22 -0300 Subject: [PATCH 4/4] workflow results --- .semaphore/semaphore.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.semaphore/semaphore.yml b/.semaphore/semaphore.yml index 6f1d00d..545441b 100644 --- a/.semaphore/semaphore.yml +++ b/.semaphore/semaphore.yml @@ -48,3 +48,10 @@ blocks: always: commands: - 'if [ -f results.xml ]; then test-results publish --name="Security checks" results.xml; fi' + +after_pipeline: + task: + jobs: + - name: Submit Reports + commands: + - test-results gen-pipeline-report \ No newline at end of file