Optimize logging in reloader #19
Conversation
|
@faizanahmad055 Yikes! You better fix it before anyone else finds out! Build 1 has Failed! |
|
@faizanahmad055 Yikes! You better fix it before anyone else finds out! Build 2 has Failed! |
|
@faizanahmad055 Image is available for testing. |
internal/pkg/handler/create.go
Outdated
| } else { | ||
| logrus.Warnf("Invalid resource: Resource should be 'Secret' or 'Configmap' but found, %v", r.Resource) | ||
| } | ||
| logrus.Errorf("Create Handler received nil object") |
There was a problem hiding this comment.
what is create handler @faizanahmad055 ?
There was a problem hiding this comment.
I have updated it to Resource creation handler received nil resource . Resource creation handler can handle the creation of new resource (i.e. Secret Or Configmap).
| @@ -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 { | |||
There was a problem hiding this comment.
why did we remove else section? why was it put in first place?
There was a problem hiding this comment.
@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.
internal/pkg/handler/update.go
Outdated
| @@ -112,6 +107,8 @@ 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 | |||
| logrus.Infof("Changes Detected in %s of type '%s' in namespace: %s", config.ResourceName, envarPostfix, config.Namespace) | |||
internal/pkg/handler/update.go
Outdated
| @@ -120,13 +117,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 did not happen for %s of type %s in namespace: %s", resourceName, upgradeFuncs.ResourceType, config.Namespace) | |||
There was a problem hiding this comment.
why didn't it happen? shouldn't we add the reason to make the log more meaningful?
internal/pkg/handler/update.go
Outdated
| @@ -23,9 +23,8 @@ 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("Update Handler received nil object") | |||
There was a problem hiding this comment.
I will suggest small "u" for Update
|
@faizanahmad055 Image is available for testing. |
|
@faizanahmad055 Image is available for testing. |
This PR handles the issue 16