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

Add CLI Upgrade command #898

Closed
wants to merge 77 commits into from
Closed

Add CLI Upgrade command #898

wants to merge 77 commits into from

Conversation

t-eckert
Copy link
Contributor

@t-eckert t-eckert commented Dec 3, 2021

This is a wrap-up of Saad's excellent work on the Upgrade command. There will need to be some refactors following this, but functionality is solid.

Changes proposed in this PR:

  • Made an initial try at the consul-k8s upgrade command. Running into issues with the connect-injector webhook not starting on an install?
  • Upgrade commit
  • First pass at upgrade was successful.
  • notes from sync with Saad on what's left
  • Made an initial try at the consul-k8s upgrade command. Running into issues with the connect-injector webhook not starting on an install?
  • Upgrade commit
  • First pass at upgrade was successful.
  • notes from sync with Saad on what's left
  • Some basic cleanup
  • Remove TestDebugger
  • Remove validateLabels (unused)
  • Add the namespace and install flags
  • Add flag test and remove install option

How I've tested this PR:

Checkout an older version of the CLI and compile it.

cd cli
git checkout tags/v0.37.0
go build -o consul-k8s-0-37-0

Checkout this version and compile it

git checkout cli-upgrade
go build -o consul-k8s-latest

Install Consul with the older version, optionally include a preset.

./consul-k8s-0-37-0 install [-preset demo]

Apply a static server/client pair to the cluster.

K8s Config
apiVersion: apps/v1
kind: Deployment
metadata:
  name: static-client
spec:
  replicas: 1
  selector:
    matchLabels:
      app: static-client
  template:
    metadata:
      name: static-client
      labels:
        app: static-client
      annotations:
        "consul.hashicorp.com/connect-inject": "true"
        "consul.hashicorp.com/connect-service-upstreams": "static-server:1234"
    spec:
      containers:
        - name: static-client
          image: docker.mirror.hashicorp.services/curlimages/curl:latest
          command: [ "/bin/sh", "-c", "--" ]
          args: [ "while true; do sleep 30; done;" ]
          env:
          - name: HOST_IP
            valueFrom:
              fieldRef:
                fieldPath: status.hostIP
      serviceAccountName: static-client
      terminationGracePeriodSeconds: 0 # so deletion is quick
---
apiVersion: v1
kind: Service
metadata:
  name: static-client
spec:
  selector:
    app: static-client
  ports:
    - port: 80
---
apiVersion: v1
kind: ServiceAccount
metadata:
  name: static-client
---
apiVersion: v1
kind: Service
metadata:
  name: static-server
spec:
  selector:
    app: static-server
  ports:
    - protocol: TCP
      port: 80
      targetPort: 8080
---
apiVersion: v1
kind: ServiceAccount
metadata:
  name: static-server
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: static-server
spec:
  replicas: 1
  selector:
    matchLabels:
      app: static-server
  template:
    metadata:
      name: static-server
      labels:
        app: static-server
      annotations:
        'consul.hashicorp.com/connect-inject': 'true'
    spec:
      containers:
        - name: static-server
          image: hashicorp/http-echo:latest
          args:
            - -text="hello world"
            - -listen=:8080
          ports:
            - containerPort: 8080
              name: http
      serviceAccountName: static-server
---
apiVersion: consul.hashicorp.com/v1alpha1
kind: ServiceIntentions
metadata:
  name: static-server
spec:
  destination:
    name: static-server
  sources:
    - name: static-client
      action: allow

Exec into the client pod

kubectl exec <client-pod-name> -it /sh

In the client pod, cURL the server pod:

curl http://localhost:1234

You should receive "hello, world" back. Exit the pod.

Now perform the upgrade with the latest version of the CLI, optionally modifying the preset.

./consul-k8s-latest upgrade [-preset secure]

Exec into the client pod again, cURL the server pod, and ensure that you still receive "hello, world".

How I expect reviewers to test this PR:

You can do what I did above. I believe in you.

Checklist:

  • Tests added
  • CHANGELOG entry added

    HashiCorp engineers only, community PRs should not add a changelog entry.
    Entries should use present tense (e.g. Add support for...)

kschoche and others added 18 commits October 20, 2021 21:16
Co-authored-by: Iryna Shustava <ishustava@users.noreply.github.com>
* Add base bootstrapping logic and acceptance tests for gossip encryption in Vault

Co-authored-by: Iryna Shustava <ishustava@users.noreply.github.com>
* Change vault cluster in acceptance tests to only run with TLS. All tests will run against vault with TLS because that is the use case we think will be the most valuable for users to test
* Support adding Vault CA as a secret to pods that will be using vault agent. We need to add two annotations to pods:
      * vault.hashicorp.com/agent-extra-secret with the value of the vault CA secret name. The secret will be mounted to vault agent at /vault/custom path. See docs here
      * vault.hashicorp.com/ca-cert - with the path of the ca file inside the vault agent container. This should be /vault/custom/<secret key>
* Most pods will only need those annotations. The server pods also need the Vault CA secret to be mounted as a volume because it needs the CA to be on the file system for the vault connect CA provider.
…ssues with the connect-injector webhook not starting on an install?
@t-eckert t-eckert marked this pull request as ready for review December 3, 2021 18:02
@t-eckert t-eckert requested review from a team, ndhanushkodi and thisisnotashwin and removed request for a team December 3, 2021 18:55
Copy link
Contributor

@ndhanushkodi ndhanushkodi left a comment

Choose a reason for hiding this comment

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

It looks great! Thanks for wrapping this up Thomas! Mostly had some small comments, and +1 to David's comment to re-enable the presets. I have a little bit of manual testing left to try but wanted to leave this review now so you can start! Finished doing some testing, haven't run into anything weird yet!

cli/cmd/upgrade/upgrade.go Outdated Show resolved Hide resolved
cli/cmd/upgrade/upgrade.go Outdated Show resolved Hide resolved
cli/cmd/upgrade/upgrade.go Outdated Show resolved Hide resolved
cli/cmd/upgrade/upgrade.go Show resolved Hide resolved
cli/cmd/upgrade/upgrade.go Show resolved Hide resolved
cli/cmd/upgrade/upgrade_test.go Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@ndhanushkodi
Copy link
Contributor

I think the acceptance tests are failing because we need to rebase this on main

@t-eckert
Copy link
Contributor Author

Closing because of weird git stuff...

@t-eckert t-eckert closed this Dec 13, 2021
@hashicorp hashicorp deleted a comment from ndhanushkodi Dec 13, 2021
@hashicorp hashicorp deleted a comment from ndhanushkodi Dec 13, 2021
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.

9 participants