kfctl delete refactor #309
Conversation
|
This change is |
|
/retest |
ccdd07d to
6f3bff5
Compare
Refactor the kfctl deletion algorithm as described in issue kubectl/kfctl#293: 1. Build kustomize folder if necessary 2. For every application in reverse KfDef order, do an equivalent of `kustomize build | kubectl delete -f -` Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>
6f3bff5 to
eede9b1
Compare
|
@adrian555 @Tomcli @kunmingg @jlewi this is ready for review. |
pkg/kfapp/kustomize/kustomize.go
Outdated
| @@ -350,36 +346,84 @@ func (kustomize *kustomize) Delete(resources kftypesv3.ResourceEnum) error { | |||
| Message: msg, | |||
| } | |||
| } | |||
| } | |||
There was a problem hiding this comment.
I wonder if line 339 to line 349 are still needed when line 361 below already handles the empty case.
There was a problem hiding this comment.
Nice catch, you're right!
pkg/kfapp/kustomize/kustomize.go
Outdated
| return &kfapisv3.KfError{ | ||
| Code: int(kfapisv3.INVALID_ARGUMENT), | ||
| Message: msg, | ||
| Code: int(kfapisv3.INTERNAL_ERROR), |
There was a problem hiding this comment.
Should we continue to delete the rest as a best effort or quit immediately?
There was a problem hiding this comment.
That's a good question 🤔
I usually think it's good to fail early.
On the other hand, kubectl tries to delete everything regardless.
We may want to stay close to kubectl behavior as in the future people will probably use it.
Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>
|
@adrian555 I made changes as per the review. |
|
Looks good, thanks @yanniszark. /lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yanniszark 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 |
* kustomize: Refactor deletion algorithm Refactor the kfctl deletion algorithm as described in issue kubectl/kfctl#293: 1. Build kustomize folder if necessary 2. For every application in reverse KfDef order, do an equivalent of `kustomize build | kubectl delete -f -` Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com> * Don't fail early, attempt to delete everything Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>
* kustomize: Refactor deletion algorithm Refactor the kfctl deletion algorithm as described in issue kubectl/kfctl#293: 1. Build kustomize folder if necessary 2. For every application in reverse KfDef order, do an equivalent of `kustomize build | kubectl delete -f -` Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com> * Don't fail early, attempt to delete everything Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>
Fixes: #293
Refactor the kfctl deletion algorithm as described in the issue.