[DEVHAS-488]annotation to force generate gitops #410
Conversation
Signed-off-by: Stephanie <[email protected]>
Signed-off-by: Stephanie <[email protected]>
Signed-off-by: Stephanie <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #410 +/- ##
==========================================
- Coverage 83.98% 81.41% -2.58%
==========================================
Files 31 32 +1
Lines 4259 4772 +513
==========================================
+ Hits 3577 3885 +308
- Misses 514 698 +184
- Partials 168 189 +21 ☔ View full report in Codecov by Sentry. |
controllers/component_controller.go
Outdated
| @@ -234,6 +243,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( | |||
| return ctrl.Result{}, err | |||
| } else { | |||
| log.Info(fmt.Sprintf("GitOps re-generation successful for %s", component.Name)) | |||
| setForceGenerateGitopsAnnotation(&component, "false") | |||
There was a problem hiding this comment.
The person or the tool that sets the annotations can handle un setting the annotation. I don't think we need a dedicated function for setting it to false.
There was a problem hiding this comment.
I'm thinking the person or HAC can enable the force generation, but they are not going to track it? and after the regeneration is done, should this be set to false? or the expected usage will be after enabling, the gitops resource going to attempt regeneration in every following reconcile?
There was a problem hiding this comment.
the expected usage will be after enabling, the gitops resource going to attempt regeneration in every following reconcile
I think the expected usage should be that as long as the annotation is present, every reconcile will re-generate the gitops resources. It's up to the user or tool to remove it.
There was a problem hiding this comment.
have discussed in team meeting, setting the annotation to false when a force generation is done
controllers/component_controller.go
Outdated
| @@ -754,3 +764,21 @@ func (r *ComponentReconciler) incrementCounterAndRequeue(log logr.Logger, ctx co | |||
| return ctrl.Result{}, componentErr | |||
| } | |||
| } | |||
|
|
|||
| // getForceGenerateGitopsAnnotation gets the internal annotation on the component whether to force generate the gitops resource | |||
| func getForceGenerateGitopsAnnotation(component appstudiov1alpha1.Component) (bool, error) { | |||
There was a problem hiding this comment.
We can simplify this a fair bit by avoiding casting the string to bool (which means we also don't need to return an err)
func getForceGenerateGitopsAnnotation(component appstudiov1alpha1.Component) bool {
if compAnnotations != nil && compAnnotations[forceGenerationAnnotation] == "true" {
return true
}
return false
}
Signed-off-by: Stephanie <[email protected]>
Signed-off-by: Stephanie <[email protected]>
Signed-off-by: Stephanie <[email protected]>
|
@johnmcollier The status update still not working. I found out any update to the CR causes the reconcile being entered twice. Since we are resetting the annotation back to I'm not sure why the reconcile is executed twice, verified I see this behavior on main as well. Assuming this is a bug, but should we hold the force annotation until that bug being fixed? or I can check the timestamp of the gitopsGeneration status in the test to verify the gitops resource has been regenerated |
|
I don’t think we need to hold the PR. The multiple reconciles is from when we set the statuses on the resource I think. I don’t think there’s a way to avoid it. I would instead introduce a new status type for the force generation, as that will not be overwritten. I was thinking about the statuses earlier, and it may be good to have two separate statuses anyways, so that we can see when the last force generation was, even if other gitops generation occurred since then. |
Signed-off-by: Stephanie <[email protected]>
Signed-off-by: Stephanie <[email protected]>
Signed-off-by: Stephanie <[email protected]>
Signed-off-by: Stephanie <[email protected]>
|
/retest |
1 similar comment
|
/retest |
|
@johnmcollier just pushed new commits to update the condition type and also controller tests. please help take another look when you have time. thx |
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: johnmcollier, yangcao77 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 |
What does this PR do?:
this PR checks for annotation
forceGitopsGenerationin component CR to force generate the gitops resources upon reconcilation being hitverified in log, once the annotation set to true, the re-generation will be triggered
Which issue(s)/story(ies) does this PR fixes:
https://issues.redhat.com/browse/DEVHAS-488
PR acceptance criteria:
Unit/Functional tests
Documentation
Client Impact
How to test changes / Special notes to the reviewer: