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

Broken ports update with multiple same number ports #7957

Closed
bozaro opened this issue Apr 21, 2020 · 7 comments
Closed

Broken ports update with multiple same number ports #7957

bozaro opened this issue Apr 21, 2020 · 7 comments

Comments

@bozaro
Copy link

bozaro commented Apr 21, 2020

Issue summary

I created simple chart with ports from values (show only related part, link to full file below):

apiVersion: apps/v1
kind: Deployment
spec:
  template:
    spec:
      containers:
        - name: "nginx"
          ports:
            {{- .Values.ports | toYaml | nindent 12 }}

And install this chart to minicube with values:

ports:
- name: http
  containerPort: 80
  protocol: TCP
- name: grpc-look-aside
  containerPort: 8080
  hostPort: 34567
  protocol: TCP
- name: grpc-service
  containerPort: 8080
  protocol: TCP

As expected I got pod with 3 ports (http, grpc-look-aside, grpc-service).

After that I upgrade chart with values (remove grpc-look-aside port):

ports:
- name: http
  containerPort: 80
  protocol: TCP
- name: grpc-service
  containerPort: 8080
  protocol: TCP

As result deployment became to unexpected state: all 8080 ports was removed.

I expected:

[
  {
    "containerPort": 80,
    "name": "http",
    "protocol": "TCP"
  },
  {
    "containerPort": 8080,
    "name": "grpc-service",
    "protocol": "TCP"
  }
]

but actual port specification was:

[
  {
    "containerPort": 80,
    "name": "http",
    "protocol": "TCP"
  }
]

How to reproduce

I created minimal chart with reproducing script: https://github.com/bozaro/helm-ports-issue

  1. checkout
  2. pip install yq
  3. ./ports.sh

Versions

Output of helm version:

version.BuildInfo{Version:"v3.1.2", GitCommit:"d878d4d45863e42fd5cff6743294a11d28a9abce", GitTreeState:"clean", GoVersion:"go1.13.8"}
version.BuildInfo{Version:"v3.2.0-rc.1", GitCommit:"7bffac813db894e06d17bac91d14ea819b5c2310", GitTreeState:"clean", GoVersion:"go1.13.10"}

Output of kubectl version:

Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.1", GitCommit:"7879fc12a63337efff607952a323df90cdc7a335", GitTreeState:"clean", BuildDate:"2020-04-14T16:45:05Z", GoVersion:"go1.13.10", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.3", GitCommit:"06ad960bfd03b39c8310aaf92d1e7c12ce618213", GitTreeState:"clean", BuildDate:"2020-02-11T18:07:13Z", GoVersion:"go1.13.6", Compiler:"gc", Platform:"linux/amd64"}

Cloud Provider/Platform (AKS, GKE, Minikube etc.):

minikube version: v1.3.1
commit: ca60a424ce69a4d79f502650199ca2b52f29e631
@technosophos
Copy link
Member

Please take a look at the Helm docs for how helm upgrade deals with values, and take a look at the flags --replace-values and --reuse-values. If you still believe that you have found a bug after that, please provide the helm commands you used (and a minimal chart) for reproducing this. https://docs.helm.sh/docs/

@bozaro
Copy link
Author

bozaro commented Apr 23, 2020

Minimal chart and helm commands is provided by repo: https://github.com/bozaro/helm-ports-issue
This repository is created specially for this issue.

@bozaro
Copy link
Author

bozaro commented Apr 23, 2020

In any case with replace and reuse values I should not lost second port declaration after upgrade.

@bozaro
Copy link
Author

bozaro commented Apr 24, 2020

This issue is not values-related. I used values only for simple reproduce.
Looks like helm on upgrade make:

  • try to apply delta between last expected state (not actual state) and new expected state;
  • on removing ports remove all ports with same number.

This is combo:

  • I adds 3 ports with number A, B, B and actual pod state became A, B, B;
  • I remove duplicate B and apply state for A, B but helm removes all B ports and state became: A;
  • all upgrades in this stage keep only A, because helm think that last actual state state is A, B.

@github-actions
Copy link

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

@github-actions
Copy link

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

@askingcat
Copy link

I just encountered this issue last week as well. Originally I used helm 3.1.0, but upgrading to 3.7.2 didn't help either.

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

No branches or pull requests

4 participants