Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up order should be reverse of install order #7284

Closed
stanislav-zaprudskiy opened this issue Apr 11, 2022 · 1 comment · Fixed by #7925
Closed

Clean up order should be reverse of install order #7284

stanislav-zaprudskiy opened this issue Apr 11, 2022 · 1 comment · Fixed by #7925
Labels
area/modules help wanted We would love to have this done, but don't have the bandwidth, need help from contributors kind/bug Something isn't working priority/p3 agreed that this would be good to have, but no one is available at the moment.

Comments

@stanislav-zaprudskiy
Copy link

Expected behavior

skaffold dev cleans up in the reverse order of its install order.

Actual behavior

skaffold dev cleans up in the same order (?) as its install order.

Information

  • Skaffold version: v1.38.0
  • Operating system: macOS 12.3.1
  • Installed via: skaffold.dev
  • Contents of skaffold.yaml:
apiVersion: skaffold/v2beta28
kind: Config
metadata:
  name: all
requires:
  - configs:
      - postgres-operator-crds
      - postgres-operator

---
apiVersion: skaffold/v2beta28
kind: Config
metadata:
  name: postgres-operator-crds
deploy:
  helm:
    flags:
      install:
        - "--skip-crds"
      upgrade:
        - "--skip-crds"
    releases:
      - name: postgres-operator-crds
        chartPath: ./helm-charts/postgres-operator-crds
        wait: true

---
apiVersion: skaffold/v2beta28
kind: Config
metadata:
  name: postgres-operator
deploy:
  helm:
    flags:
      install:
        - "--skip-crds"
      upgrade:
        - "--skip-crds"
    releases:
      - name: postgres-operator
        remoteChart: postgres-operator
        repo: https://opensource.zalando.com/postgres-operator/charts/postgres-operator/
        version: 1.7.1
        namespace: awx
        createNamespace: true
        wait: true

Steps to reproduce the behavior

The idea is that CRDs are installed first, and then the actual CRs are deployed - which works fine for the initial installation. However, when clean up of skaffold dev runs, it seems to be executing clean up in the same order as install: first postgres-operator-crds is removed, but then the removal of postgres-operator fails as CRDs aren't there anymore.

$ skaffold dev --module all
Listing files to watch...
Generating tags...
Checking cache...
Tags used in deployment:
Starting deploy...
Helm release postgres-operator-crds not installed. Installing...
NAME: postgres-operator-crds
LAST DEPLOYED: Mon Apr 11 21:18:24 2022
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None
Helm release postgres-operator not installed. Installing...
manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
NAME: postgres-operator
LAST DEPLOYED: Mon Apr 11 21:18:34 2022
NAMESPACE: awx
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
To verify that postgres-operator has started, run:

  kubectl --namespace=awx get pods -l "app.kubernetes.io/name=postgres-operator"
Waiting for deployments to stabilize...
 - awx:deployment/postgres-operator is ready.
Deployments stabilized in 1.229 second
Press Ctrl+C to exit
Watching for changes...
^CCleaning up...
release "postgres-operator-crds" uninstalled
Error: uninstallation completed with 1 error(s): unable to build kubernetes objects for delete: unable to recognize "": no matches for kind "OperatorConfiguration" in version "acid.zalan.do/v1"
WARN[0030] deployer cleanup:exit status 1                subtask=-1 task=DevLoop

Originally I tried just using releases list within same module for both, but it fails similarly on clean up.

@MarlonGamez MarlonGamez added area/modules kind/bug Something isn't working priority/p2 May take a couple of releases labels Apr 12, 2022
@tejal29
Copy link
Contributor

tejal29 commented Apr 13, 2022

makes complete sense.

@tejal29 tejal29 added priority/p3 agreed that this would be good to have, but no one is available at the moment. help wanted We would love to have this done, but don't have the bandwidth, need help from contributors and removed priority/p2 May take a couple of releases labels Sep 12, 2022
tejal29 pushed a commit that referenced this issue Oct 19, 2022
* fix: reverse order of deployers during cleanup (#7284)

* chore: add .tool-versions to gitignore

.tool-versions is used by asdf (package manager) to set the go version
for this repository

Co-authored-by: Thomas Steinert <hello@chroni.cc>
aaron-prindle pushed a commit that referenced this issue Feb 6, 2023
…7927)

* chore: add .tool-versions to gitignore

.tool-versions is used by asdf (package manager) to set the go version
for this repository

* fix: reverse order of deployers during cleanup (#7284)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/modules help wanted We would love to have this done, but don't have the bandwidth, need help from contributors kind/bug Something isn't working priority/p3 agreed that this would be good to have, but no one is available at the moment.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants