replication: add new Validated condition #664
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
Rakshith-R
force-pushed
the
failed-precondition
branch
from
f6e223e to
ab2f4c1
Compare
This commit adds new Validated condition. This is initially used to indicate the csi driver responded with FailedPrecondition grpc code for EnableReplication request using `PrerequisiteNotMet` reason. Signed-off-by: Rakshith R <[email protected]>
Rakshith-R
force-pushed
the
failed-precondition
branch
from
ab2f4c1 to
39c304c
Compare
Rakshith-R
changed the title
Madhu-1
reviewed
Sep 12, 2024
| if resp.HasKnownGRPCError(enableReplicationKnownErrors) { | ||
| setFailedValidationCondition(&vr.instance.Status.Conditions, vr.instance.Generation) | ||
| } else { | ||
| setFailedPromotionCondition(&vr.instance.Status.Conditions, vr.instance.Generation) |
There was a problem hiding this comment.
can you please use setFailureCondition(instance) as it internally takes care of setting the same.
There was a problem hiding this comment.
I would like to use similar format of using different functions currently present in status.go than forcing another function (for just one case) just to handle if else decision which then would again call the functions present in status.go .
There was a problem hiding this comment.
we already have a generic helper function to set the conditions i don't like to see others calling it even though we have a helper function to do it. and for this again it's a bug we should not be setting setFailedPromotionCondition condition if the enable replication fails, the condition should reflect what went wrong.
There was a problem hiding this comment.
we already have a generic helper function to set the conditions i don't like to see others calling it even though we have a helper function to do it. and for this again it's a bug we should not be setting
setFailedPromotionConditioncondition if the enable replication fails, the condition should reflect what went wrong.
Enabling replication is part of promotion. I don't see a issue with that part.
Why do you want to force a helper function that is used at a lot of other places to fit into a new scenario ?
If we do that, we'll need more arguments and the function will not be so generic anymore.
There was a problem hiding this comment.
even thought its part of the condition we should reflect on what exactly went wrong #666 is the tracker for it.
Why do you want to force a helper function that is used at a lot of other places to fit into a new scenario ?
If we do that, we'll need more arguments and the function will not be so generic anymore.
i don't see any new change in the behaviour to call setFailureCondition instead of setFailedPromotionCondition
There was a problem hiding this comment.
I really don't get why you want to call that function when you already know which function it will in turn call.
There are a lot of places already calling functions from status.go directly.
There was a problem hiding this comment.
we need to remove all that one and remove unwanted functions, we should be doing call A in one function to set status and call B in one function to set status, we need have only one way to set the conditions
mergify
bot
requested review from
nixpanic,
ShyamsundarR,
black-dragon74,
iPraveenParihar and
yati1998
Madhu-1
reviewed
Sep 12, 2024
Madhu-1
approved these changes
Sep 12, 2024
iPraveenParihar
approved these changes
Sep 12, 2024
lumiere-bot bot
referenced
this pull request
in coolguy1771/home-ops
Sep 19, 2024
… ) (#5423) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [kubernetes-csi-addons](https://redirect.github.com/csi-addons/kubernetes-csi-addons) | minor | `v0.9.1` -> `v0.10.0` | --- ### Release Notes <details> <summary>csi-addons/kubernetes-csi-addons (kubernetes-csi-addons)</summary> ### [`v0.10.0`](https://redirect.github.com/csi-addons/kubernetes-csi-addons/releases/tag/v0.10.0) [Compare Source](https://redirect.github.com/csi-addons/kubernetes-csi-addons/compare/v0.9.1...v0.10.0) #### What's Changed - Add proto and sidecar code for volumegroup by [@​Nikhil-Ladha](https://redirect.github.com/Nikhil-Ladha) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/652](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/652) - vendor: Bump google.golang.org/grpc from 1.65.0 to 1.66.0 in the golang-dependencies group by [@​dependabot](https://redirect.github.com/dependabot) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/656](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/656) - vendor: Bump sigs.k8s.io/controller-tools from 0.16.1 to 0.16.2 in /tools in the k8s-dependencies group by [@​dependabot](https://redirect.github.com/dependabot) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/658](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/658) - vendor: Bump the github-dependencies group across 1 directory with 3 updates by [@​dependabot](https://redirect.github.com/dependabot) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/657](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/657) - replication: add new Validated condition by [@​Rakshith-R](https://redirect.github.com/Rakshith-R) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/664](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/664) - bundle: Update CSV capability to Seamless Upgrades by [@​black-dragon74](https://redirect.github.com/black-dragon74) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/668](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/668) - Refactor PVC controller by [@​black-dragon74](https://redirect.github.com/black-dragon74) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/662](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/662) - bundle: Add missing CRDs to bundle CSV by [@​black-dragon74](https://redirect.github.com/black-dragon74) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/669](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/669) - vendor: bump the k8s-dependencies group with 3 updates by [@​dependabot](https://redirect.github.com/dependabot) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/673](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/673) #### New Contributors - [@​Nikhil-Ladha](https://redirect.github.com/Nikhil-Ladha) made their first contribution in [https://github.com/csi-addons/kubernetes-csi-addons/pull/652](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/652) **Full Changelog**: csi-addons/kubernetes-csi-addons@v0.9.1...v0.10.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://redirect.github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC44NS4wIiwidXBkYXRlZEluVmVyIjoiMzguODUuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsicmVub3ZhdGUvZ2l0aHViLXJlbGVhc2UiLCJ0eXBlL21pbm9yIl19--> Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
spiceratops
referenced
this pull request
in spiceratops/k8s-gitops
Sep 26, 2024
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [kubernetes-csi-addons](https://redirect.github.com/csi-addons/kubernetes-csi-addons) | minor | `v0.9.1` -> `v0.10.0` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>csi-addons/kubernetes-csi-addons (kubernetes-csi-addons)</summary> ### [`v0.10.0`](https://redirect.github.com/csi-addons/kubernetes-csi-addons/releases/tag/v0.10.0) [Compare Source](https://redirect.github.com/csi-addons/kubernetes-csi-addons/compare/v0.9.1...v0.10.0) #### What's Changed - Add proto and sidecar code for volumegroup by [@​Nikhil-Ladha](https://redirect.github.com/Nikhil-Ladha) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/652](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/652) - vendor: Bump google.golang.org/grpc from 1.65.0 to 1.66.0 in the golang-dependencies group by [@​dependabot](https://redirect.github.com/dependabot) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/656](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/656) - vendor: Bump sigs.k8s.io/controller-tools from 0.16.1 to 0.16.2 in /tools in the k8s-dependencies group by [@​dependabot](https://redirect.github.com/dependabot) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/658](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/658) - vendor: Bump the github-dependencies group across 1 directory with 3 updates by [@​dependabot](https://redirect.github.com/dependabot) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/657](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/657) - replication: add new Validated condition by [@​Rakshith-R](https://redirect.github.com/Rakshith-R) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/664](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/664) - bundle: Update CSV capability to Seamless Upgrades by [@​black-dragon74](https://redirect.github.com/black-dragon74) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/668](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/668) - Refactor PVC controller by [@​black-dragon74](https://redirect.github.com/black-dragon74) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/662](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/662) - bundle: Add missing CRDs to bundle CSV by [@​black-dragon74](https://redirect.github.com/black-dragon74) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/669](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/669) - vendor: bump the k8s-dependencies group with 3 updates by [@​dependabot](https://redirect.github.com/dependabot) in [https://github.com/csi-addons/kubernetes-csi-addons/pull/673](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/673) #### New Contributors - [@​Nikhil-Ladha](https://redirect.github.com/Nikhil-Ladha) made their first contribution in [https://github.com/csi-addons/kubernetes-csi-addons/pull/652](https://redirect.github.com/csi-addons/kubernetes-csi-addons/pull/652) **Full Changelog**: csi-addons/kubernetes-csi-addons@v0.9.1...v0.10.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://redirect.github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC44NS4wIiwidXBkYXRlZEluVmVyIjoiMzguODUuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsicmVub3ZhdGUvZ2l0aHViLXJlbGVhc2UiLCJ0eXBlL21pbm9yIl19-->
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.
This commit adds new Validated condition.
This is initially used to indicate the csi driver
responded with FailedPrecondition grpc code for
EnableReplication request using
PrerequisiteNotMetreason.