Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Optimize logging in reloader #19

Merged
merged 5 commits into from
Aug 2, 2018
Merged

Optimize logging in reloader #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
af7e722
Optimize logging in reloader
faizanahmad055 Aug 1, 2018
8c89f1a
Fix test case failing issue
faizanahmad055 Aug 1, 2018
0aa2878
Implement PR-19 review comments
faizanahmad055 Aug 2, 2018
d41a38e
Place the log out of loop
faizanahmad055 Aug 2, 2018
c26ed7e
Fix change detection log
faizanahmad055 Aug 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/pkg/constants/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ package constants

const (
// ConfigmapEnvVarPostfix is a postfix for configmap envVar
ConfigmapEnvVarPostfix = "_CONFIGMAP"
ConfigmapEnvVarPostfix = "CONFIGMAP"
// SecretEnvVarPostfix is a postfix for secret envVar
SecretEnvVarPostfix = "_SECRET"
SecretEnvVarPostfix = "SECRET"
// EnvVarPrefix is a Prefix for environment variable
EnvVarPrefix = "STAKATER_"
)
10 changes: 0 additions & 10 deletions internal/pkg/controller/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ var (

func TestMain(m *testing.M) {

logrus.Infof("Creating namespace %s", namespace)
testutil.CreateNamespace(namespace, client)

logrus.Infof("Creating controller")
Expand All @@ -45,7 +44,6 @@ func TestMain(m *testing.M) {
logrus.Infof("Running Testcases")
retCode := m.Run()

logrus.Infof("Deleting namespace %q.\n", namespace)
testutil.DeleteNamespace(namespace, client)

os.Exit(retCode)
Expand Down Expand Up @@ -268,7 +266,6 @@ func TestControllerUpdatingSecretShouldCreateEnvInDeployment(t *testing.T) {
if !updated {
t.Errorf("Deployment was not updated")
}
//time.Sleep(5 * time.Second)

// Deleting Deployment
err = testutil.DeleteDeployment(client, namespace, secretName)
Expand Down Expand Up @@ -330,7 +327,6 @@ func TestControllerUpdatingSecretShouldUpdateEnvInDeployment(t *testing.T) {
if !updated {
t.Errorf("Deployment was not updated")
}
//time.Sleep(5 * time.Second)

// Deleting Deployment
err = testutil.DeleteDeployment(client, namespace, secretName)
Expand Down Expand Up @@ -385,7 +381,6 @@ func TestControllerUpdatingSecretLabelsShouldNotCreateorUpdateEnvInDeployment(t
if updated {
t.Errorf("Deployment should not be updated by changing label in secret")
}
//time.Sleep(5 * time.Second)

// Deleting Deployment
err = testutil.DeleteDeployment(client, namespace, secretName)
Expand Down Expand Up @@ -559,7 +554,6 @@ func TestControllerUpdatingSecretShouldCreateEnvInDaemonSet(t *testing.T) {
if !updated {
t.Errorf("DaemonSet was not updated")
}
//time.Sleep(5 * time.Second)

// Deleting DaemonSet
err = testutil.DeleteDaemonSet(client, namespace, secretName)
Expand Down Expand Up @@ -622,7 +616,6 @@ func TestControllerUpdatingSecretShouldUpdateEnvInDaemonSet(t *testing.T) {
if !updated {
t.Errorf("DaemonSet was not updated")
}
//time.Sleep(5 * time.Second)

// Deleting DaemonSet
err = testutil.DeleteDaemonSet(client, namespace, secretName)
Expand Down Expand Up @@ -677,7 +670,6 @@ func TestControllerUpdatingSecretLabelsShouldNotCreateorUpdateEnvInDaemonSet(t *
if updated {
t.Errorf("DaemonSet should not be updated by changing label in secret")
}
//time.Sleep(5 * time.Second)

// Deleting DaemonSet
err = testutil.DeleteDaemonSet(client, namespace, secretName)
Expand Down Expand Up @@ -851,7 +843,6 @@ func TestControllerUpdatingSecretShouldCreateEnvInStatefulSet(t *testing.T) {
if !updated {
t.Errorf("StatefulSet was not updated")
}
//time.Sleep(5 * time.Second)

// Deleting StatefulSet
err = testutil.DeleteStatefulSet(client, namespace, secretName)
Expand Down Expand Up @@ -913,7 +904,6 @@ func TestControllerUpdatingSecretShouldUpdateEnvInStatefulSet(t *testing.T) {
if !updated {
t.Errorf("StatefulSet was not updated")
}
//time.Sleep(5 * time.Second)

// Deleting StatefulSet
err = testutil.DeleteStatefulSet(client, namespace, secretName)
Expand Down
13 changes: 1 addition & 12 deletions internal/pkg/handler/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package handler

import (
"github.com/sirupsen/logrus"
"k8s.io/api/core/v1"
)

// ResourceCreatedHandler contains new objects
Expand All @@ -13,17 +12,7 @@ type ResourceCreatedHandler struct {
// Handle processes the newly created resource
func (r ResourceCreatedHandler) Handle() error {
if r.Resource == nil {
logrus.Errorf("Error in Handler")
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did we remove else section? why was it put in first place?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rasheedamir , Else section was put there to show a log statement that a new resource (i.e. secret or configmap) has been added. But it does not add any value to the reloader functionality because reloader should only show the logs when a resource has been updated.

logrus.Infof("Detected changes in object %s", r.Resource)
// process resource based on its type
if _, ok := r.Resource.(*v1.ConfigMap); ok {
logrus.Infof("A 'configmap' has been 'Added' but no implementation found to take action")
} else if _, ok := r.Resource.(*v1.Secret); ok {
logrus.Infof("A 'secret' has been 'Added' but no implementation found to take action")
} else {
logrus.Warnf("Invalid resource: Resource should be 'Secret' or 'Configmap' but found, %v", r.Resource)
}
logrus.Errorf("Resource creation handler received nil resource")
}
return nil
}
74 changes: 33 additions & 41 deletions internal/pkg/handler/update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,60 +23,55 @@ type ResourceUpdatedHandler struct {
// Handle processes the updated resource
func (r ResourceUpdatedHandler) Handle() error {
if r.Resource == nil || r.OldResource == nil {
logrus.Errorf("Error in Handler")
logrus.Errorf("Resource update handler received nil resource")
} else {
logrus.Infof("Detected changes in object %s", r.Resource)
// process resource based on its type
rollingUpgrade(r, callbacks.RollingUpgradeFuncs{
ItemsFunc: callbacks.GetDeploymentItems,
ContainersFunc: callbacks.GetDeploymentContainers,
UpdateFunc: callbacks.UpdateDeployment,
ResourceType: "Deployment",
})
rollingUpgrade(r, callbacks.RollingUpgradeFuncs{
ItemsFunc: callbacks.GetDaemonSetItems,
ContainersFunc: callbacks.GetDaemonSetContainers,
UpdateFunc: callbacks.UpdateDaemonSet,
ResourceType: "DaemonSet",
})
rollingUpgrade(r, callbacks.RollingUpgradeFuncs{
ItemsFunc: callbacks.GetStatefulSetItems,
ContainersFunc: callbacks.GetStatefulsetContainers,
UpdateFunc: callbacks.UpdateStatefulset,
ResourceType: "StatefulSet",
})
config, envVarPostfix, oldSHAData := getConfig(r)
if config.SHAValue != oldSHAData {
logrus.Infof("Changes detected in %s of type '%s' in namespace: %s", config.ResourceName, envVarPostfix, config.Namespace)
// process resource based on its type
rollingUpgrade(r, config, envVarPostfix, callbacks.RollingUpgradeFuncs{
ItemsFunc: callbacks.GetDeploymentItems,
ContainersFunc: callbacks.GetDeploymentContainers,
UpdateFunc: callbacks.UpdateDeployment,
ResourceType: "Deployment",
})
rollingUpgrade(r, config, envVarPostfix, callbacks.RollingUpgradeFuncs{
ItemsFunc: callbacks.GetDaemonSetItems,
ContainersFunc: callbacks.GetDaemonSetContainers,
UpdateFunc: callbacks.UpdateDaemonSet,
ResourceType: "DaemonSet",
})
rollingUpgrade(r, config, envVarPostfix, callbacks.RollingUpgradeFuncs{
ItemsFunc: callbacks.GetStatefulSetItems,
ContainersFunc: callbacks.GetStatefulsetContainers,
UpdateFunc: callbacks.UpdateStatefulset,
ResourceType: "StatefulSet",
})
}
}
return nil
}

func rollingUpgrade(r ResourceUpdatedHandler, upgradeFuncs callbacks.RollingUpgradeFuncs) {
func rollingUpgrade(r ResourceUpdatedHandler, config util.Config, envarPostfix string, upgradeFuncs callbacks.RollingUpgradeFuncs) {
client, err := kube.GetClient()
if err != nil {
logrus.Fatalf("Unable to create Kubernetes client error = %v", err)
}

config, envVarPostfix, oldSHAData := getConfig(r)

if config.SHAValue != oldSHAData {
err = PerformRollingUpgrade(client, config, envVarPostfix, upgradeFuncs)
if err != nil {
logrus.Fatalf("Rolling upgrade failed with error = %v", err)
}
} else {
logrus.Infof("Rolling upgrade will not happend because no actual change in data has been detected")
err = PerformRollingUpgrade(client, config, envarPostfix, upgradeFuncs)
if err != nil {
logrus.Errorf("Rolling upgrade for %s failed with error = %v", config.ResourceName, err)
}
}

func getConfig(r ResourceUpdatedHandler) (util.Config, string, string) {
var oldSHAData, envVarPostfix string
var config util.Config
if _, ok := r.Resource.(*v1.ConfigMap); ok {
logrus.Infof("Performing 'Updated' action for resource of type 'configmap'")
oldSHAData = getSHAfromConfigmap(r.OldResource.(*v1.ConfigMap).Data)
config = getConfigmapConfig(r)
envVarPostfix = constants.ConfigmapEnvVarPostfix
} else if _, ok := r.Resource.(*v1.Secret); ok {
logrus.Infof("Performing 'Updated' action for resource of type 'secret'")
oldSHAData = getSHAfromSecret(r.OldResource.(*v1.Secret).Data)
config = getSecretConfig(r)
envVarPostfix = constants.SecretEnvVarPostfix
Expand Down Expand Up @@ -112,6 +107,7 @@ func PerformRollingUpgrade(client kubernetes.Interface, config util.Config, enva
var err error
for _, i := range items {
containers := upgradeFuncs.ContainersFunc(i)
resourceName := util.ToObjectMeta(i).Name
// find correct annotation and update the resource
annotationValue := util.ToObjectMeta(i).Annotations[config.Annotation]
if annotationValue != "" {
Expand All @@ -120,13 +116,13 @@ func PerformRollingUpgrade(client kubernetes.Interface, config util.Config, enva
if value == config.ResourceName {
updated := updateContainers(containers, value, config.SHAValue, envarPostfix)
if !updated {
logrus.Warnf("Rolling upgrade did not happen")
logrus.Warnf("Rolling upgrade failed because no container found to add environment variable in %s of type %s in namespace: %s", resourceName, upgradeFuncs.ResourceType, config.Namespace)
} else {
err = upgradeFuncs.UpdateFunc(client, config.Namespace, i)
if err != nil {
logrus.Errorf("Update %s failed %v", upgradeFuncs.ResourceType, err)
logrus.Errorf("Update for %s of type %s in namespace %s failed with error %v", resourceName, upgradeFuncs.ResourceType, config.Namespace, err)
} else {
logrus.Infof("Updated %s of type %s", config.ResourceName, upgradeFuncs.ResourceType)
logrus.Infof("Updated %s of type %s in namespace: %s ", resourceName, upgradeFuncs.ResourceType, config.Namespace)
}
break
}
Expand All @@ -139,8 +135,7 @@ func PerformRollingUpgrade(client kubernetes.Interface, config util.Config, enva

func updateContainers(containers []v1.Container, annotationValue string, shaData string, envarPostfix string) bool {
updated := false
envar := constants.EnvVarPrefix + util.ConvertToEnvVarName(annotationValue) + envarPostfix
logrus.Infof("Generated environment variable: %s", envar)
envar := constants.EnvVarPrefix + util.ConvertToEnvVarName(annotationValue)+ "_" + envarPostfix
for i := range containers {
envs := containers[i].Env

Expand All @@ -155,7 +150,6 @@ func updateContainers(containers []v1.Container, annotationValue string, shaData
}
containers[i].Env = append(containers[i].Env, e)
updated = true
logrus.Infof("%s environment variable does not exist, creating a new envVar", envar)
}
}
return updated
Expand All @@ -164,9 +158,7 @@ func updateContainers(containers []v1.Container, annotationValue string, shaData
func updateEnvVar(envs []v1.EnvVar, envar string, shaData string) bool {
for j := range envs {
if envs[j].Name == envar {
logrus.Infof("%s environment variable found", envar)
if envs[j].Value != shaData {
logrus.Infof("Updating %s", envar)
envs[j].Value = shaData
return true
}
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/handler/update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var (

func TestMain(m *testing.M) {

logrus.Infof("Creating namespace %s", namespace)
// Creating namespace
testutil.CreateNamespace(namespace, client)

logrus.Infof("Setting up the test resources")
Expand Down
3 changes: 1 addition & 2 deletions internal/pkg/testutil/kube.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,6 @@ func GetResourceSHA(containers []v1.Container, envar string) string {
//ConvertResourceToSHA generates SHA from secret or configmap data
func ConvertResourceToSHA(resourceType string, namespace string, resourceName string, data string) string {
values := []string{}
logrus.Infof("Generating SHA for secret data")
if resourceType == SecretResourceType {
secret := GetSecret(namespace, resourceName, data)
for k, v := range secret.Data {
Expand Down Expand Up @@ -391,7 +390,7 @@ func VerifyResourceUpdate(client kubernetes.Interface, config util.Config, envVa
}
}
if matches {
envName := constants.EnvVarPrefix + util.ConvertToEnvVarName(annotationValue) + envVarPostfix
envName := constants.EnvVarPrefix + util.ConvertToEnvVarName(annotationValue) + "_" + envVarPostfix
updated := GetResourceSHA(containers, envName)

if updated == config.SHAValue {
Expand Down