Separate positive polarity conditions for ArtifactInStorage #646
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
aryan9600
reviewed
Mar 30, 2022
darkowlzz
force-pushed
the
positive-conditions
branch
from
f78a225 to
aff9030
Compare
pjbgf
reviewed
Mar 30, 2022
darkowlzz
force-pushed
the
positive-conditions
branch
from
aff9030 to
c10f4c0
Compare
pjbgf
reviewed
Mar 30, 2022
There was a problem hiding this comment.
@darkowlzz nice work! I added a nit and a question, apart from that it LGTM.
Introduce separate positive polarity conditions which are used to set Ready condition. Move the "artifact stored" ready condition into ArtifactInStorage positive polarity condition. If ArtifactInStorage is True and there's no negative polarity condition present, the Ready condition is summarized with ArtifactInStorage condition value. Also, update the priorities of the conditions. ArtifactInStorage has higher priority than SourceVerfied condition. If both are present, the Ready condition will have ArtifactInStorage. The negative polarity conditions are reordered to have the most likely actual cause of failure condition the highest priority, for example StorageOperationFailed, followed by the conditions that are reconciled first in the whole reconciliation so as to prioritize the first failure which may be the cause of subsequent failures. Signed-off-by: Sunny <darkowlzz@protonmail.com>
Update alll the other reconcilers similar to the GitRepository reconcilers to introduce positive condition ArtifactInStorage and reorder the status conditions. Signed-off-by: Sunny <darkowlzz@protonmail.com>
Signed-off-by: Sunny <darkowlzz@protonmail.com>
Signed-off-by: Sunny <darkowlzz@protonmail.com>
darkowlzz
force-pushed
the
positive-conditions
branch
from
c10f4c0 to
b869716
Compare
|
For reference, with this change, the status conditions of GitRepository object after a successful reconciliation with source verification enabled would look like: conditions:
- lastTransitionTime: "2022-03-29T20:42:13Z"
message: stored artifact for revision 'no-source-ignore/60c7ae324d2edbb692ebd82d744c2e3cecc141f4'
observedGeneration: 1
reason: Succeeded
status: "True"
type: Ready
- lastTransitionTime: "2022-03-29T20:42:13Z"
message: stored artifact for revision 'no-source-ignore/60c7ae324d2edbb692ebd82d744c2e3cecc141f4'
observedGeneration: 1
reason: Succeeded
status: "True"
type: ArtifactInStorage
- lastTransitionTime: "2022-03-29T20:42:13Z"
message: verified signature of commit '60c7ae324d2edbb692ebd82d744c2e3cecc141f4'
observedGeneration: 1
reason: Succeeded
status: "True"
type: SourceVerified
observedGeneration: 1And when there's a failure due to misconfiguration in the new generation, the conditions would look like: conditions:
- lastTransitionTime: "2022-03-30T11:34:05Z"
message: reconciling new object generation (2)
observedGeneration: 2
reason: NewGeneration
status: "True"
type: Reconciling
- lastTransitionTime: "2022-03-30T11:34:10Z"
message: 'failed to checkout and determine revision: unable to clone ''https://github.com/darkowlzz/flux2-kustomize-helm-example2'':
too many redirects or authentication replays'
observedGeneration: 2
reason: GitOperationFailed
status: "False"
type: Ready
- lastTransitionTime: "2022-03-30T11:34:10Z"
message: 'failed to checkout and determine revision: unable to clone ''https://github.com/darkowlzz/flux2-kustomize-helm-example2'':
too many redirects or authentication replays'
observedGeneration: 2
reason: GitOperationFailed
status: "True"
type: FetchFailed
- lastTransitionTime: "2022-03-29T20:42:13Z"
message: stored artifact for revision 'no-source-ignore/60c7ae324d2edbb692ebd82d744c2e3cecc141f4'
observedGeneration: 1
reason: Succeeded
status: "True"
type: ArtifactInStorage
- lastTransitionTime: "2022-03-29T20:42:13Z"
message: verified signature of commit '60c7ae324d2edbb692ebd82d744c2e3cecc141f4'
observedGeneration: 1
reason: Succeeded
status: "True"
type: SourceVerified
observedGeneration: 1After the resolution of the above failure: conditions:
- lastTransitionTime: "2022-03-30T11:34:28Z"
message: stored artifact for revision 'no-source-ignore/60c7ae324d2edbb692ebd82d744c2e3cecc141f4'
observedGeneration: 3
reason: Succeeded
status: "True"
type: Ready
- lastTransitionTime: "2022-03-29T20:42:13Z"
message: stored artifact for revision 'no-source-ignore/60c7ae324d2edbb692ebd82d744c2e3cecc141f4'
observedGeneration: 3
reason: Succeeded
status: "True"
type: ArtifactInStorage
- lastTransitionTime: "2022-03-29T20:42:13Z"
message: verified signature of commit '60c7ae324d2edbb692ebd82d744c2e3cecc141f4'
observedGeneration: 3
reason: Succeeded
status: "True"
type: SourceVerified
observedGeneration: 3Similar changes in all the other Source kinds. |
pjbgf
approved these changes
Mar 30, 2022
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.
Introduce separate positive polarity conditions which are used to set
Ready condition. Move the "artifact stored" ready condition into
ArtifactInStorage positive polarity condition. If ArtifactInStorage is
True and there's no negative polarity condition present, the Ready
condition is summarized with ArtifactInStorage condition value.
Also, update the priorities of the conditions. ArtifactInStorage has
higher priority than SourceVerfied condition. If both are present, the
Ready condition will have ArtifactInStorage.
The negative polarity conditions are reordered to have the most likely
actual cause of failure condition the highest priority, for example
StorageOperationFailed, followed by the conditions that are reconciled
first in the whole reconciliation so as to prioritize the first failure
which may be the cause of subsequent failures.
In addition, the "artifact up-to-date" log is now also emitted as an event.
Fixes #617