Rework orphaned Templates removal #571
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eromanova
force-pushed
the
rework-templates-removal
branch
from
6f76152 to
691e70e
Compare
zerospiel
reviewed
Nov 5, 2024
There was a problem hiding this comment.
I left a couple of nit comments along with a couple of questions.
And one more question: why do we implement the full reconciliation test logic in the BeforeEach and AfterEach nodes? The logic in those might interfere with other tests (e.g. removing a namespace in one of the AfterEach node, whilst the same namespace is being utilized by another node from another test). This question does not require any actions to perform, I am just curious about it
eromanova
force-pushed
the
rework-templates-removal
branch
from
8b32df0 to
b12a2f3
Compare
|
Very big thanks for the review @zerospiel. Regarding your question about the reconciliation tests logic, I think we should create a task for it and continue the discussion there. I'd agree, the current reconciliation tests approach may be reworked. |
eromanova
force-pushed
the
rework-templates-removal
branch
from
b12a2f3 to
d617632
Compare
zerospiel
previously approved these changes
Nov 6, 2024
Kshatrix
reviewed
Nov 7, 2024
eromanova
force-pushed
the
rework-templates-removal
branch
2 times, most recently
from
7e3e42b to
c59fc89
Compare
zerospiel
previously approved these changes
Nov 7, 2024
Kshatrix
requested changes
Nov 8, 2024
eromanova
force-pushed
the
rework-templates-removal
branch
from
c59fc89 to
b85ed80
Compare
eromanova
force-pushed
the
rework-templates-removal
branch
3 times, most recently
from
0b34261 to
955ccfc
Compare
Closes k0rdent#570 Includes: * Remove managed-by-chain label from templates since one template can be managed by multiple chains * Trigger TemplateChain reconciliation on TemplateManagement updates * Use cached k8s client in testing for operations that require field indexers since field indexers are only work with cached client * Separate TemplateChain reconciliation to Update and Delete operations
eromanova
force-pushed
the
rework-templates-removal
branch
from
955ccfc to
e2acc45
Compare
eromanova
force-pushed
the
rework-templates-removal
branch
from
e2acc45 to
3566f95
Compare
Kshatrix
approved these changes
Nov 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #570
Includes:
managed-by-chainlabel from templates since one template can be managed by multiple chains and this label is not needed anymore