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

Add Events for far Remediation Process #107

Merged
merged 8 commits into from
Jan 3, 2024
Merged

Add Events for far Remediation Process #107

merged 8 commits into from
Jan 3, 2024

Conversation

razo7
Copy link
Member

@razo7 razo7 commented Nov 22, 2023 •
edited
Loading

To ease the tracking of the remediation process of far custom resource we add events to indicate major milestones (such as fencing, workload deletion, etc.)

  1. Adding events (and unit testing)
  2. Check whether NoExecute taint was added to signal an event.
  3. Only use GetNodeWithName instead of IsNodeNameValid which is no longer needed.
  4. Small refactoring to unit tests

ECOPROJECT-1663

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 22, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@razo7
Copy link
Member Author

razo7 commented Dec 13, 2023

/test 4.14-openshift-e2e
/test 4.15-openshift-e2e

clobrano
clobrano reviewed Dec 14, 2023
View reviewed changes
pkg/utils/nodes.go Outdated Show resolved Hide resolved
@razo7
Copy link
Member Author

razo7 commented Dec 14, 2023

/test 4.15-openshift-e2e

@razo7
Copy link
Member Author

razo7 commented Dec 17, 2023

/test 4.14-openshift-e2e
/test 4.15-openshift-e2e

@razo7 razo7 mentioned this pull request Dec 17, 2023
Merged
@razo7
Copy link
Member Author

razo7 commented Dec 18, 2023

/test 4.15-openshift-e2e

@clobrano
Copy link
Contributor

May I ask you to wait for #106 before merging this? Many events come from the fence agent execution that is reworked in that PR.

@razo7
Copy link
Member Author

razo7 commented Dec 31, 2023

/test 4.14-openshift-e2e
/test 4.15-openshift-e2e

clobrano
clobrano requested changes Jan 2, 2024
View reviewed changes
Copy link
Contributor

@clobrano clobrano left a comment

Choose a reason for hiding this comment

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

I left some comments

controllers/fenceagentsremediation_controller_test.go Outdated Show resolved Hide resolved
controllers/fenceagentsremediation_controller_test.go Outdated Show resolved Hide resolved
controllers/fenceagentsremediation_controller_test.go Outdated Show resolved Hide resolved
@razo7
Copy link
Member Author

razo7 commented Jan 2, 2024

/test 4.14-openshift-e2e
/test 4.15-openshift-e2e

@clobrano
Copy link
Contributor

clobrano commented Jan 2, 2024

/test 4.14-openshift-e2e

@razo7
Copy link
Member Author

razo7 commented Jan 2, 2024

/test 4.14-openshift-e2e
/test 4.15-openshift-e2e

@openshift-ci openshift-ci bot added the lgtm label Jan 3, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Jan 3, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: clobrano, razo7

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@razo7 razo7 marked this pull request as ready for review January 3, 2024 15:00
@openshift-ci openshift-ci bot removed the lgtm label Jan 3, 2024
@razo7
Fetch events API from commons
467480b 
Use Medik8s shared events
@razo7
Add events to far CR for better visibility of the remediation process
49bc54b 
The events will help to identify the remediation process by looking on far CR events
@razo7
Add events to unhealthy node for better visibility of the remediation…
5712863 
… process

The events will help to identify the remediation process by looking on node events. Fetch node from IsNodeNameValid, and whether taint was added by AppendTaint
@razo7
Only use GetNodeWithName func
d738a52 
IsNodeNameValid is no longer needed, as we no longer only need to check CR node validty but we want to receive the node object as well on success
@razo7
Typo fix & CLI errors
2cb984f 
s/CreateFARRemediationTaint/CreateRemediationTaint. Look for the actual errors, so we can catch a change in the actual errors from the UTs
@razo7
Refactor far unit-test
a55dc08 
Use human readable timing for timeouts with even better var names. Use verifyPreRemediationSucceed that include checking for finalizer and taint (from event and value). Sleep for a second between unit tests to give dummy reconcilation to run first.
@razo7
Fix typo
8e72dad 
s/reconicliation/reconciliation
@razo7
Pass namespace to function
34ce0ee 
Decouple test
@clobrano
Copy link
Contributor

clobrano commented Jan 3, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jan 3, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 9fd53ef into medik8s:main Jan 3, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mshitrit mshitrit mshitrit left review comments

@clobrano clobrano Awaiting requested review from clobrano

@slintes slintes Awaiting requested review from slintes

Assignees

@clobrano clobrano

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@razo7 @clobrano @mshitrit @openshift-merge-robot