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

Logs and events fixes #174

Merged
merged 4 commits into from
Jan 17, 2024
Merged

Logs and events fixes #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b7307c5
ECOPROJECT-1826 - multiple messages of "Automatically selected OutOfS…
mshitrit Jan 14, 2024
96d6d17
Reorder jobs in full-gen in order for it to work properly on vendor p…
mshitrit Jan 15, 2024
af81a44
ECOPROJECT-1827
mshitrit Jan 15, 2024
953ded5
ECOPROJECT-1829
mshitrit Jan 15, 2024
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
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,4 +446,4 @@ fix-imports: sort-imports ## Sort imports
$(SORT_IMPORTS) -w .

.PHONY: full-gen
full-gen: generate manifests vendor tidy bundle fix-imports bundle-reset ## generates all automatically generated content
full-gen: tidy vendor generate manifests bundle fix-imports bundle-reset ## generates all automatically generated content
30 changes: 19 additions & 11 deletions controllers/selfnoderemediation_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"time"

"github.com/go-logr/logr"
"github.com/medik8s/common/pkg/events"
"github.com/medik8s/common/pkg/resources"
"github.com/pkg/errors"

Expand Down Expand Up @@ -49,7 +50,6 @@ const (
nhcTimeOutAnnotation = "remediation.medik8s.io/nhc-timed-out"
excludeRemediationLabel = "remediation.medik8s.io/exclude-from-remediation"

eventReasonRemediationCreated = "RemediationCreated"
eventReasonRemediationStopped = "RemediationStopped"
eventReasonRemediationSkipped = "RemediationSkipped"

Expand Down Expand Up @@ -175,7 +175,10 @@ func (r *SelfNodeRemediationReconciler) Reconcile(ctx context.Context, req ctrl.
return ctrl.Result{}, err
}

r.Recorder.Event(snr, eventTypeNormal, eventReasonRemediationCreated, "Remediation started")
//used as an indication not to spam the event
if isFinalizerAlreadyAdded := controllerutil.ContainsFinalizer(snr, SNRFinalizer); !isFinalizerAlreadyAdded {
events.RemediationStarted(r.Recorder, snr)
}

defer func() {
if updateErr := r.updateSnrStatus(ctx, snr); updateErr != nil {
Expand All @@ -195,9 +198,8 @@ func (r *SelfNodeRemediationReconciler) Reconcile(ctx context.Context, req ctrl.
}()

if r.isStoppedByNHC(snr) {
msg := "SNR remediation was stopped by Node Healthcheck"
r.logger.Info(msg)
r.Recorder.Event(snr, eventTypeNormal, eventReasonRemediationStopped, msg)
r.logger.Info("NHC added the timed-out annotation, remediation will be stopped")
events.RemediationStoppedByNHC(r.Recorder, snr)
return ctrl.Result{}, r.updateConditions(remediationTimeoutByNHC, snr)
}

Expand Down Expand Up @@ -230,7 +232,7 @@ func (r *SelfNodeRemediationReconciler) Reconcile(ctx context.Context, req ctrl.
return ctrl.Result{}, nil
}

strategy := r.getRuntimeStrategy(snr.Spec.RemediationStrategy)
strategy := r.getRuntimeStrategy(snr)
switch strategy {
case v1alpha1.ResourceDeletionRemediationStrategy:
result, err = r.remediateWithResourceDeletion(snr, node)
Expand Down Expand Up @@ -483,6 +485,7 @@ func (r *SelfNodeRemediationReconciler) recoverNode(node *v1.Node, snr *v1alpha1
return ctrl.Result{}, err
}
r.Recorder.Event(snr, eventTypeNormal, eventReasonRemoveFinalizer, "Remediation process - remove finalizer from snr")
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this use events.NormalEvent from common?

Copy link
Member

Choose a reason for hiding this comment

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

IIRC the common events PR have been added after SNR events PR was merged. I guess we didn't update SNR with the common events API

Copy link
Contributor

Choose a reason for hiding this comment

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

I was asking becase in the line below we use events.RemediationFinished, which should come from the common events API

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, TBH I didn't think of that .
I've only considered the "special" events.
I'll update in another PR

Copy link
Member Author

Choose a reason for hiding this comment

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

Here: #178

events.RemediationFinished(r.Recorder, snr)
}

return ctrl.Result{}, nil
Expand Down Expand Up @@ -897,18 +900,23 @@ func (r *SelfNodeRemediationReconciler) isResourceDeletionExpired(snr *v1alpha1.
return true, 0
}

func (r *SelfNodeRemediationReconciler) getRuntimeStrategy(strategy v1alpha1.RemediationStrategyType) v1alpha1.RemediationStrategyType {
func (r *SelfNodeRemediationReconciler) getRuntimeStrategy(snr *v1alpha1.SelfNodeRemediation) v1alpha1.RemediationStrategyType {
strategy := snr.Spec.RemediationStrategy
if strategy != v1alpha1.AutomaticRemediationStrategy {
return strategy
}

remediationStrategy := v1alpha1.ResourceDeletionRemediationStrategy
if utils.IsOutOfServiceTaintGA {
r.logger.Info("Automatically selected OutOfServiceTaint Remediation strategy")
return v1alpha1.OutOfServiceTaintRemediationStrategy
remediationStrategy = v1alpha1.OutOfServiceTaintRemediationStrategy
}

//used as an indication not to spam the log
if isFinalizerAlreadyAdded := controllerutil.ContainsFinalizer(snr, SNRFinalizer); !isFinalizerAlreadyAdded {
r.logger.Info(fmt.Sprintf("Automatically selected %s Remediation strategy", remediationStrategy))
}

r.logger.Info("Automatically selected ResourceDeletion Remediation strategy")
return v1alpha1.ResourceDeletionRemediationStrategy
return remediationStrategy

}

Expand Down
2 changes: 1 addition & 1 deletion controllers/tests/controller/selfnoderemediation_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ var _ = Describe("SNR Controller", func() {
It("Remediation flow", func() {
node := verifyNodeIsUnschedulable()

verifyEvent("Normal", "RemediationCreated", "Remediation started")
verifyEvent("Normal", "RemediationStarted", "[remediation] Remediation started")

verifyEvent("Normal", "MarkUnschedulable", "Remediation process - unhealthy node marked as unschedulable")

Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
github.com/Masterminds/sprig v2.22.0+incompatible
github.com/go-logr/logr v1.2.3
github.com/go-ping/ping v1.1.0
github.com/medik8s/common v1.10.0
github.com/medik8s/common v1.12.0
github.com/onsi/ginkgo/v2 v2.9.1
github.com/onsi/gomega v1.27.4
github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 // release-4.13
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ github.com/mailru/easyjson v0.7.6/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJ
github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
github.com/matttproud/golang_protobuf_extensions v1.0.2 h1:hAHbPm5IJGijwng3PWk09JkG9WeqChjprR5s9bBZ+OM=
github.com/matttproud/golang_protobuf_extensions v1.0.2/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4=
github.com/medik8s/common v1.10.0 h1:ghvW/Kiv9xdkJbFhzeh38dSmx0ltoZNT7cHrCh3WpX4=
github.com/medik8s/common v1.10.0/go.mod h1:ZT/3hfMXJLmZEcqmxRWB5LGC8Wl+qKGGQ4zM8hOE7PY=
github.com/medik8s/common v1.12.0 h1:UJ5VS4rbo/K0snfuqRiYam1NhXTwo4Q7fTng34YSlmA=
github.com/medik8s/common v1.12.0/go.mod h1:Q6YR2rgyiLl6ob4Mx2uDBmybAB3d94fImwoHPbeiE54=
github.com/mitchellh/copystructure v1.1.2 h1:Th2TIvG1+6ma3e/0/bopBKohOTY7s4dA8V2q4EUcBJ0=
github.com/mitchellh/copystructure v1.1.2/go.mod h1:EBArHfARyrSWO/+Wyr9zwEkc6XMFB9XyNgFNmRkZZU4=
github.com/mitchellh/reflectwalk v1.0.1 h1:FVzMWA5RllMAKIdUSC8mdWo3XtwoecrH79BY70sEEpE=
Expand Down
51 changes: 51 additions & 0 deletions vendor/github.com/medik8s/common/pkg/events/events.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions vendor/github.com/medik8s/common/pkg/labels/labels.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion vendor/modules.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,9 @@ github.com/mailru/easyjson/jwriter
# github.com/matttproud/golang_protobuf_extensions v1.0.2
## explicit; go 1.9
github.com/matttproud/golang_protobuf_extensions/pbutil
# github.com/medik8s/common v1.10.0
# github.com/medik8s/common v1.12.0
## explicit; go 1.20
github.com/medik8s/common/pkg/events
github.com/medik8s/common/pkg/labels
github.com/medik8s/common/pkg/nodes
github.com/medik8s/common/pkg/resources
Expand Down