feat: delete wait refreshing pr #8345
Conversation
Signed-off-by: pashavictorovich <pavel@codefresh.io>
Signed-off-by: pashavictorovich <pavel@codefresh.io>
Codecov Report
@@ Coverage Diff @@
## master #8345 +/- ##
==========================================
- Coverage 42.80% 42.27% -0.54%
==========================================
Files 186 176 -10
Lines 23306 22894 -412
==========================================
- Hits 9976 9678 -298
+ Misses 11902 11833 -69
+ Partials 1428 1383 -45
Continue to review full report at Codecov.
|
Signed-off-by: pashavictorovich <pavel@codefresh.io>
Signed-off-by: pashavictorovich <pavel@codefresh.io>
|
@pasha-codefresh Could you please resolve the conflicts? |
|
yes @chetan-rns , will try solve it today |
…te-wait-v Signed-off-by: pashavictorovich <pavel@codefresh.io> # Conflicts: # cmd/argocd/commands/app.go
|
@chetan-rns merged, could you please review PR? |
Signed-off-by: pashavictorovich <pavel@codefresh.io>
| } | ||
| selectedResources := parseSelectedResources(resources) | ||
| if !watch.sync && !watch.health && !watch.operation && !watch.suspended && !watch.delete { |
There was a problem hiding this comment.
A minor nit:
| if !watch.sync && !watch.health && !watch.operation && !watch.suspended && !watch.delete { | |
| if (watch == watchOpts{}) { |
| client: acdClient, | ||
| appName: appName, | ||
| timeout: timeout, | ||
| watch: watchOpts{operation: true}, |
There was a problem hiding this comment.
Since we are waiting for the application to sync is it necessary to enable sync in the watchOpts?
| return nil, fmt.Errorf("application '%s' health state has transitioned from %s to %s", opts.appName, prevState.Health, newState.Health) | ||
| } | ||
| if opts.watch.delete && newState.Health == string(health.HealthStatusMissing) { | ||
| newState.Message = strings.Replace(newState.Message, "created", "deleted", 1) |
There was a problem hiding this comment.
Sometimes the wait command prints that the resource is deleted even though it is only being synced. In the below message, the deployment.apps/test is shown as deleted during sync
2022-03-03T14:18:38+05:30 Service test test OutOfSync Missing
2022-03-03T14:18:57+05:30 ServiceAccount test test Synced Missing
2022-03-03T14:18:57+05:30 Service test test Synced Healthy
2022-03-03T14:18:57+05:30 ServiceAccount test test Synced Missing serviceaccount/test created
2022-03-03T14:18:57+05:30 Service test test Synced Healthy service/test created
2022-03-03T14:18:57+05:30 apps Deployment test test OutOfSync Missing deployment.apps/test deleted
2022-03-03T14:18:57+05:30 apps Deployment test test Synced Progressing deployment.apps/test created
2022-03-03T14:19:08+05:30 apps Deployment test test Synced Healthy deployment.apps/test createdSteps to reproduce:
- Create an app with manual sync
- argocd app wait test --delete
- Sync the application
| if opts.watch.delete { | ||
| _, err := appClient.Get(context.Background(), &applicationpkg.ApplicationQuery{Name: &opts.appName}) | ||
| if err != nil { | ||
| if strings.HasSuffix(err.Error(), "not found") { |
There was a problem hiding this comment.
| if strings.HasSuffix(err.Error(), "not found") { | |
| if apierr.IsNotFound(err) { |
Maybe using IsNotFound(err) error type check from "k8s.io/apimachinery/pkg/api/errors" is a better option here?
|
@chetan-rns i have decided close this pr and reopen it with smaller parts ( it was just reopening of stale pr ). After your review I understand that it may include few bugs so I think splitting it and deliver with more tests and better code will be correct decision here |
I have reopened #2575 due to inactivity in original PR
Closes: #2555