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

#38: add uninstall command #891

Closed
wants to merge 4 commits into from
Closed

Conversation

ipolyzos
Copy link
Contributor

No description provided.

Copy link
Contributor

@oscerd oscerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one too is wrong.

@ipolyzos
Copy link
Contributor Author

rebased and pushed

Copy link
Contributor

@jamesnetherton jamesnetherton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some comments for a few things I noticed.

Would also be good to have an e2e test to prove that the uninstall command is working as desired.

pkg/cmd/uninstall.go Outdated Show resolved Hide resolved
pkg/cmd/uninstall.go Outdated Show resolved Hide resolved
pkg/cmd/uninstall.go Outdated Show resolved Hide resolved
@ipolyzos
Copy link
Contributor Author

thank you @jamesnetherton, I have pushed the changes

Copy link
Member

@dmvolod dmvolod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some resources have not deleted after uninstall
[dvolodin@dvolodin camel-k]$ oc get all,pvc,configmap,rolebindings,clusterrolebindings,secrets,sa,roles,clusterroles,crd -l 'app=camel-k'
NAME TYPE FROM LATEST
buildconfig.build.openshift.io/camel-k-kit-bkvghlo2qg41c2hgbbkg Source Binary 1

NAME TYPE FROM STATUS STARTED DURATION
build.build.openshift.io/camel-k-kit-bkvghlo2qg41c2hgbbkg-1 Source Binary Complete 2 days ago 9s

NAME DOCKER REPO TAGS UPDATED
imagestream.image.openshift.io/camel-k-kit-bkvghlo2qg41c2hgbbkg 172.30.1.1:5000/myproject/camel-k-kit-bkvghlo2qg41c2hgbbkg 948792 2 days ago

NAME DATA AGE
configmap/camel-k-maven-settings 1 2d

NAME AGE
rolebinding.rbac.authorization.k8s.io/camel-k-operator 2d

NAME SECRETS AGE
serviceaccount/camel-k-operator 2 2d

NAME AGE
role.rbac.authorization.k8s.io/camel-k-operator 2d

integrations pods become orphans without integration definition
[dvolodin@dvolodin camel-k]$ ./kamel get
Error: no matches for kind "Integration" in version "camel.apache.org/v1alpha1"

[dvolodin@dvolodin camel-k]$ oc get pods
NAME READY STATUS RESTARTS AGE
camel-k-kit-bkvghlo2qg41c2hgbbkg-1-build 0/1 Completed 0 2d
greetings-6cdc5d9ffd-bdp8r 1/1 Running 0 10m

@jamesnetherton
Copy link
Contributor

Please run lint checks as the CI build reports a number of failures on this change.

@ipolyzos
Copy link
Contributor Author

ipolyzos commented Aug 2, 2019

@dmvolod and @jamesnetherton thank you for your input. I have gone through the changes and applied to the pull request. Please see below the manual lint and resource uninstall verification procedure logs.

lint log


$ make lint                                                                                                                                             git:(issue-38|) 
GOGC=10 golangci-lint run --verbose --deadline 5m
INFO [config_reader] Config search paths: [./ /home/ipolyzos/repositories/camel-k /home/ipolyzos/repositories /home/ipolyzos /home /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 29 linters: [bodyclose deadcode depguard errcheck goconst gocritic gocyclo gofmt goimports golint gosec gosimple govet ineffassign interfacer lll maligned misspell nakedret prealloc scopelint staticcheck structcheck stylecheck typecheck unconvert unparam unused varcheck] 
INFO [lintersdb] Optimized sublinters [staticcheck gosimple unused stylecheck] into metalinter megacheck 
INFO [loader] Go packages loading at mode load types and syntax took 5.880706962s 
INFO [loader] SSA repr building timing: packages building 132.97403ms, total 1.055513s 
INFO [runner] worker.11 took 5.477250445s with stages: ineffassign: 3.672461275s, scopelint: 653.432924ms, gocyclo: 551.474018ms, goconst: 497.324917ms, structcheck: 83.059513ms, errcheck: 19.457507ms 
INFO [runner] worker.4 took 5.540390022s with stages: unparam: 5.030804253s, lll: 219.699039ms, varcheck: 121.043627ms, unconvert: 90.786136ms, deadcode: 63.259828ms, maligned: 9.440717ms, nakedret: 5.313918ms, typecheck: 4.406µs 
INFO [runner] worker.9 took 5.738099311s with stages: misspell: 5.738089721s 
INFO [runner] worker.10 took 5.775880483s with stages: gosec: 5.41534402s, prealloc: 360.522495ms 
INFO [runner] worker.6 took 6.354804193s with stages: gocritic: 6.354794223s 
INFO [runner] worker.7 took 6.362987297s with stages: gofmt: 6.36297663s 
INFO [runner] worker.12 took 7.105409723s with stages: bodyclose: 7.105383396s 
INFO [runner] worker.2 took 7.161804319s with stages: govet: 7.161780954s 
INFO [runner] worker.3 took 7.243569757s with stages: interfacer: 7.243527005s, depguard: 18.897µs 
INFO [runner] worker.5 took 7.476312853s with stages: goimports: 7.476306306s 
INFO [runner] worker.8 took 8.282600115s with stages: golint: 8.282590033s 
INFO [runner] worker.1 took 13.19004728s with stages: megacheck: 13.19003964s 
INFO [runner] Workers idle times: #2: 6.026103676s, #3: 5.934139125s, #4: 7.647264675s, #5: 5.707658026s, #6: 6.829568269s, #7: 6.804755021s, #8: 4.905310032s, #9: 7.42390483s, #10: 7.371597497s, #11: 7.66840712s, #12: 6.082184852s 
INFO [runner] Issues before processing: 67, after processing: 0 
INFO [runner] processing took 53.930282ms with stages: identifier_marker: 26.584502ms, exclude: 13.208169ms, nolint: 5.308432ms, autogenerated_exclude: 4.703422ms, skip_dirs: 2.810557ms, cgo: 634.304µs, path_prettifier: 563.414µs, filename_unadjuster: 99.829µs, max_from_linter: 3.079µs, path_shortener: 2.448µs, diff: 2.439µs, source_code: 2.108µs, exclude-rules: 1.97µs, max_same_issues: 1.691µs, uniq_by_line: 1.515µs, skip_files: 1.27µs, max_per_file_from_linter: 1.133µs 
INFO File cache stats: 231 entries of total size 939.8KiB 
INFO Memory: 140 samples, avg is 1794.3MB, max is 3504.4MB 

resource uninstall log

$ kamel install                                                                                                                                         git:(issue-38|) 
Camel K installed in namespace default
$ kubectl get all,pvc,configmap,rolebindings,clusterrolebindings,secrets,sa,roles,clusterroles,crd -l 'app=camel-k'                                     git:(issue-38|) 
NAME                               READY   UP-TO-DATE   AVAILABLE   AGE
deployment.apps/camel-k-operator   0/1     1            0           12s

NAME                                                     AGE
rolebinding.rbac.authorization.k8s.io/camel-k-operator   12s

NAME                              SECRETS   AGE
serviceaccount/camel-k-operator   1         12s

NAME                                              AGE
role.rbac.authorization.k8s.io/camel-k-operator   12s

NAME                                                 AGE
clusterrole.rbac.authorization.k8s.io/camel-k:edit   12s

NAME                                                                                  CREATED AT
customresourcedefinition.apiextensions.k8s.io/builds.camel.apache.org                 2019-08-02T09:50:52Z
customresourcedefinition.apiextensions.k8s.io/camelcatalogs.camel.apache.org          2019-08-02T09:50:52Z
customresourcedefinition.apiextensions.k8s.io/integrationkits.camel.apache.org        2019-08-02T09:50:52Z
customresourcedefinition.apiextensions.k8s.io/integrationplatforms.camel.apache.org   2019-08-02T09:50:52Z
customresourcedefinition.apiextensions.k8s.io/integrations.camel.apache.org           2019-08-02T09:50:52Z
$ kamel run examples/simple.js                                                                                                                          git:(issue-38|) 
integration "simple" updated
$ kamel run examples/greetings.groovy                                                                                                                   git:(issue-38|) 
integration "greetings" updated
$ kamel get                                                                                                                                             git:(issue-38|) 
NAME            PHASE   CONTEXT
greetings
simple
$ kubectl get all,pvc,configmap,rolebindings,clusterrolebindings,secrets,sa,roles,clusterroles,crd -l 'app=camel-k'                                     git:(issue-38|) 
NAME                               READY   UP-TO-DATE   AVAILABLE   AGE
deployment.apps/camel-k-operator   0/1     1            0           2m26s

NAME                                                     AGE
rolebinding.rbac.authorization.k8s.io/camel-k-operator   2m26s

NAME                              SECRETS   AGE
serviceaccount/camel-k-operator   1         2m26s

NAME                                              AGE
role.rbac.authorization.k8s.io/camel-k-operator   2m26s

NAME                                                 AGE
clusterrole.rbac.authorization.k8s.io/camel-k:edit   2m26s

NAME                                                                                  CREATED AT
customresourcedefinition.apiextensions.k8s.io/builds.camel.apache.org                 2019-08-02T09:50:52Z
customresourcedefinition.apiextensions.k8s.io/camelcatalogs.camel.apache.org          2019-08-02T09:50:52Z
customresourcedefinition.apiextensions.k8s.io/integrationkits.camel.apache.org        2019-08-02T09:50:52Z
customresourcedefinition.apiextensions.k8s.io/integrationplatforms.camel.apache.org   2019-08-02T09:50:52Z
customresourcedefinition.apiextensions.k8s.io/integrations.camel.apache.org           2019-08-02T09:50:52Z
$ kamel uninstall                                                                                                                                       git:(issue-38|) 
Camel-K Integration Platform removed from namespace default
Camel-K Custom Resource Definitions removed from namespace default
Camel-K Role Bindings removed from namespace default
Camel-K Roles removed from namespace default
Camel-K Cluster Roles removed from namespace default
Camel-K Service Accounts removed from namespace default
Camel-K Cluster Wide Resources removed from namespace default
Camel-K Operator removed from namespace default
$ kubectl get all,pvc,configmap,rolebindings,clusterrolebindings,secrets,sa,roles,clusterroles,crd -l 'app=camel-k'                                     git:(issue-38|) 
No resources found.
$ kamel get                                                                                                                                             git:(issue-38|) 
Error: no matches for kind "Integration" in version "camel.apache.org/v1alpha1"
Usage:
  kamel get [flags]

Flags:
  -h, --help   help for get

Global Flags:
      --config string      Path to the config file to use for CLI requests
  -n, --namespace string   Namespace to use for all operations

Error: no matches for kind "Integration" in version "camel.apache.org/v1alpha1"

@dmvolod
Copy link
Member

dmvolod commented Aug 2, 2019

@ipolyzos CI env again fails. Please see details.

@ipolyzos
Copy link
Contributor Author

ipolyzos commented Aug 2, 2019

I can not replicate the issue with go mod tidy in localhost.

Please see the log below

$ export GO111MODULE="on"
$ gimme version                                                                                                                                             git:(master|) 
v1.5.3
$ go version                                                                                                                                                git:(master|) 
go version go1.12.7 linux/amd64
$ go mod tidy

@ipolyzos
Copy link
Contributor Author

ipolyzos commented Aug 2, 2019

I tried go mod tidy from inside docker instances in order to simulate clean builds i.e docker run --rm -it --privileged -v $PWD:/try -w /try golang:latest /bin/bash. I have noticed a few long pauses while trying to find some packages. I wonder if this may be causing some timeout failure in Travis.

P.S go mod tidy succeeds however ...

@oscerd
Copy link
Contributor

oscerd commented Jan 23, 2020

What's the status here?

@ipolyzos
Copy link
Contributor Author

ipolyzos commented Feb 4, 2020

I left some comments for a few things I noticed.

Would also be good to have an e2e test to prove that the uninstall command is working as desired.

e2e tests added if you please review

@ipolyzos
Copy link
Contributor Author

ipolyzos commented Feb 4, 2020

I think this one too is wrong.

if you please review the most recent changes

@ipolyzos ipolyzos closed this Feb 4, 2020
@ipolyzos
Copy link
Contributor Author

ipolyzos commented Feb 4, 2020

reopening

@ipolyzos ipolyzos reopened this Feb 4, 2020
@jamesnetherton
Copy link
Contributor

CI tests for the uninstall steps failed. But I verified locally and it was all ok.

I forget that on Travis, the integration tests run as a non-privileged user and does not have permission to delete cluster resources like CRDs etc.

To get this work merged, maybe for now we comment out or skip the uninstall tests and I'll follow up with a fix later on.

@oscerd
Copy link
Contributor

oscerd commented Feb 5, 2020

CI tests for the uninstall steps failed. But I verified locally and it was all ok.

I forget that on Travis, the integration tests run as a non-privileged user and does not have permission to delete cluster resources like CRDs etc.

To get this work merged, maybe for now we comment out or skip the uninstall tests and I'll follow up with a fix later on.

+1

@ipolyzos
Copy link
Contributor Author

ipolyzos commented Feb 5, 2020

CI tests for the uninstall steps failed. But I verified locally and it was all ok.
I forget that on Travis, the integration tests run as a non-privileged user and does not have permission to delete cluster resources like CRDs etc.
To get this work merged, maybe for now we comment out or skip the uninstall tests and I'll follow up with a fix later on.

+1

thank you

@jamesnetherton
Copy link
Contributor

Superseded by #1254.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants