From 4353ec1ff434c08c7b98bc29540e920cc6f8ba13 Mon Sep 17 00:00:00 2001 From: Nikolas De Giorgis Date: Wed, 26 Aug 2020 11:05:40 +0100 Subject: [PATCH 1/5] add WithAgentFlags function --- pkg/kube/container/containers.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/kube/container/containers.go b/pkg/kube/container/containers.go index 5820a8248..17cf4e2f3 100644 --- a/pkg/kube/container/containers.go +++ b/pkg/kube/container/containers.go @@ -152,3 +152,16 @@ func WithSecurityContext(context corev1.SecurityContext) Modification { container.SecurityContext = &context } } + +// WithAgentFlags takes a slice of envVars corresponding to +// pairs of key-value agent startup flags and concatenates them +// into a single string that is then passed as env variable AGENT_FLAGS +func WithAgentFlags(envs ...corev1.EnvVar) Modification { + return func(container *corev1.Container) { + agentParams := "" + for _, variable := range envs { + agentParams += " -" + variable.Name + " " + variable.Value + } + container.Env = envvar.MergeWithOverride(container.Env, []corev1.EnvVar{{Name: "AGENT_FLAGS", Value: agentParams}}) + } +} From c313f605339becf4ca130b3333ac00d6010ebed7 Mon Sep 17 00:00:00 2001 From: Nikolas De Giorgis Date: Wed, 26 Aug 2020 11:49:25 +0100 Subject: [PATCH 2/5] PR feedback --- cmd/versionhook/main.go | 28 +++++++++++------------ pkg/agent/agentflags.go | 19 +++++++++++++++ pkg/{agenthealth => agent}/agenthealth.go | 2 +- 3 files changed, 34 insertions(+), 15 deletions(-) create mode 100644 pkg/agent/agentflags.go rename pkg/{agenthealth => agent}/agenthealth.go (98%) diff --git a/cmd/versionhook/main.go b/cmd/versionhook/main.go index dec13942b..11ddcf3f2 100644 --- a/cmd/versionhook/main.go +++ b/cmd/versionhook/main.go @@ -11,7 +11,7 @@ import ( "github.com/pkg/errors" - "github.com/mongodb/mongodb-kubernetes-operator/pkg/agenthealth" + "github.com/mongodb/mongodb-kubernetes-operator/pkg/agent" "go.uber.org/zap" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -91,7 +91,7 @@ func setupLogger() *zap.SugaredLogger { // waitForAgentHealthStatus will poll the health status file and wait for it to be updated. // The agent doesn't write the plan to the file right away and hence we need to wait for the // latest plan to be written. -func waitForAgentHealthStatus() (agenthealth.Health, error) { +func waitForAgentHealthStatus() (agent.Health, error) { ticker := time.NewTicker(pollingInterval) defer ticker.Stop() @@ -104,12 +104,12 @@ func waitForAgentHealthStatus() (agenthealth.Health, error) { health, err := getAgentHealthStatus() if err != nil { - return agenthealth.Health{}, err + return agent.Health{}, err } status, ok := health.Healthiness[getHostname()] if !ok { - return agenthealth.Health{}, errors.Errorf("couldn't find status for hostname %s", getHostname()) + return agent.Health{}, errors.Errorf("couldn't find status for hostname %s", getHostname()) } // We determine if the file has been updated by checking if the process is not in goal state. @@ -118,30 +118,30 @@ func waitForAgentHealthStatus() (agenthealth.Health, error) { return health, nil } } - return agenthealth.Health{}, errors.Errorf("agent health status not ready after waiting %s", pollingDuration.String()) + return agent.Health{}, errors.Errorf("agent health status not ready after waiting %s", pollingDuration.String()) } -// getAgentHealthStatus returns an instance of agenthealth.Health read +// getAgentHealthStatus returns an instance of agent.Health read // from the health file on disk -func getAgentHealthStatus() (agenthealth.Health, error) { +func getAgentHealthStatus() (agent.Health, error) { f, err := os.Open(os.Getenv(agentStatusFilePathEnv)) if err != nil { - return agenthealth.Health{}, errors.Errorf("could not open file: %s", err) + return agent.Health{}, errors.Errorf("could not open file: %s", err) } defer f.Close() h, err := readAgentHealthStatus(f) if err != nil { - return agenthealth.Health{}, errors.Errorf("could not read health status file: %s", err) + return agent.Health{}, errors.Errorf("could not read health status file: %s", err) } return h, err } // readAgentHealthStatus reads an instance of health.Health from the provided // io.Reader -func readAgentHealthStatus(reader io.Reader) (agenthealth.Health, error) { - var h agenthealth.Health +func readAgentHealthStatus(reader io.Reader) (agent.Health, error) { + var h agent.Health data, err := ioutil.ReadAll(reader) if err != nil { return h, err @@ -157,7 +157,7 @@ func getHostname() string { // shouldDeletePod returns a boolean value indicating if this pod should be deleted // this would be the case if the agent is currently trying to upgrade the version // of mongodb. -func shouldDeletePod(health agenthealth.Health) (bool, error) { +func shouldDeletePod(health agent.Health) (bool, error) { status, ok := health.ProcessPlans[getHostname()] if !ok { return false, errors.Errorf("hostname %s was not in the process plans", getHostname()) @@ -169,7 +169,7 @@ func shouldDeletePod(health agenthealth.Health) (bool, error) { // on the mongod pod to be restarted. In order to do this, we need to check the agent // status file and determine if the mongod has been stopped and if we are in the process // of a version change. -func isWaitingToBeDeleted(healthStatus agenthealth.MmsDirectorStatus) bool { +func isWaitingToBeDeleted(healthStatus agent.MmsDirectorStatus) bool { if len(healthStatus.Plans) == 0 { return false } @@ -223,7 +223,7 @@ func getThisPod() (corev1.Pod, error) { func inClusterClient() (client.Client, error) { config, err := rest.InClusterConfig() if err != nil { - return nil, errors.Errorf("could not get cluster config: %%s", err) + return nil, errors.Errorf("could not get cluster config: %s", err) } k8sClient, err := client.New(config, client.Options{}) diff --git a/pkg/agent/agentflags.go b/pkg/agent/agentflags.go new file mode 100644 index 000000000..c1a3e325b --- /dev/null +++ b/pkg/agent/agentflags.go @@ -0,0 +1,19 @@ +package agent + +import corev1 "k8s.io/api/core/v1" + +type StartupParameter struct { + Key string `json:"key"` + Value string `json:"value"` +} + +// WithAgentFlags takes a slice of envVars corresponding to +// pairs of key-value agent startup flags and concatenates them +// into a single string that is then passed as env variable AGENT_FLAGS +func StartupParametersToAgentFlag(parameters ...StartupParameter) corev1.EnvVar { + agentParams := "" + for _, param := range parameters { + agentParams += " -" + param.Key + " " + param.Value + } + return corev1.EnvVar{Name: "AGENT_FLAGS", Value: agentParams} +} diff --git a/pkg/agenthealth/agenthealth.go b/pkg/agent/agenthealth.go similarity index 98% rename from pkg/agenthealth/agenthealth.go rename to pkg/agent/agenthealth.go index 5c95e2d93..7bd16c435 100644 --- a/pkg/agenthealth/agenthealth.go +++ b/pkg/agent/agenthealth.go @@ -1,4 +1,4 @@ -package agenthealth +package agent import ( "time" From 6ca54bda273e711ae917cb6ee443302509a8c8a0 Mon Sep 17 00:00:00 2001 From: Nikolas De Giorgis Date: Wed, 26 Aug 2020 12:00:15 +0100 Subject: [PATCH 3/5] fixed comment --- pkg/agent/agentflags.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/agent/agentflags.go b/pkg/agent/agentflags.go index c1a3e325b..ed231da09 100644 --- a/pkg/agent/agentflags.go +++ b/pkg/agent/agentflags.go @@ -7,9 +7,9 @@ type StartupParameter struct { Value string `json:"value"` } -// WithAgentFlags takes a slice of envVars corresponding to -// pairs of key-value agent startup flags and concatenates them -// into a single string that is then passed as env variable AGENT_FLAGS +// StartupParametersToAgentFlag takes a slice of StartupParameters +// and concatenates them into a single string that is then +// returned as env variable AGENT_FLAGS func StartupParametersToAgentFlag(parameters ...StartupParameter) corev1.EnvVar { agentParams := "" for _, param := range parameters { From 0b24558639cbe53093280974f73a200f0e46a328 Mon Sep 17 00:00:00 2001 From: Nikolas De Giorgis Date: Wed, 26 Aug 2020 12:01:51 +0100 Subject: [PATCH 4/5] removed function --- pkg/kube/container/containers.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/pkg/kube/container/containers.go b/pkg/kube/container/containers.go index 17cf4e2f3..5820a8248 100644 --- a/pkg/kube/container/containers.go +++ b/pkg/kube/container/containers.go @@ -152,16 +152,3 @@ func WithSecurityContext(context corev1.SecurityContext) Modification { container.SecurityContext = &context } } - -// WithAgentFlags takes a slice of envVars corresponding to -// pairs of key-value agent startup flags and concatenates them -// into a single string that is then passed as env variable AGENT_FLAGS -func WithAgentFlags(envs ...corev1.EnvVar) Modification { - return func(container *corev1.Container) { - agentParams := "" - for _, variable := range envs { - agentParams += " -" + variable.Name + " " + variable.Value - } - container.Env = envvar.MergeWithOverride(container.Env, []corev1.EnvVar{{Name: "AGENT_FLAGS", Value: agentParams}}) - } -} From df7fca22ab5ec88e77546518a0f333cd6d02f36c Mon Sep 17 00:00:00 2001 From: Nikolas De Giorgis Date: Wed, 26 Aug 2020 12:24:44 +0100 Subject: [PATCH 5/5] Added basic unit testing --- pkg/agent/agentflags_test.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 pkg/agent/agentflags_test.go diff --git a/pkg/agent/agentflags_test.go b/pkg/agent/agentflags_test.go new file mode 100644 index 000000000..e56b67613 --- /dev/null +++ b/pkg/agent/agentflags_test.go @@ -0,0 +1,34 @@ +package agent + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAgentFlagIsCorrectlyCreated(t *testing.T) { + parameters := []StartupParameter{ + { + Key: "Key1", + Value: "Value1", + }, + { + Key: "Key2", + Value: "Value2", + }, + } + + envVar := StartupParametersToAgentFlag(parameters...) + assert.Equal(t, "AGENT_FLAGS", envVar.Name) + assert.Equal(t, " -Key1 Value1 -Key2 Value2", envVar.Value) + +} + +func TestAgentFlagEmptyParameters(t *testing.T) { + parameters := []StartupParameter{} + + envVar := StartupParametersToAgentFlag(parameters...) + assert.Equal(t, "AGENT_FLAGS", envVar.Name) + assert.Equal(t, "", envVar.Value) + +}