[stable/prometheus] migrate API versions from deprecated, removed versions #17268

oke-py · 2019-09-19T22:59:51Z

Is this a new chart

NOTE: We're experiencing a high volume of PRs to this repo and reviews will be delayed. Please host your own chart repository and submit your repository to the Helm Hub instead of this repo to make them discoverable to the community. Here is how to submit new chart repositories to the Helm Hub.

What this PR does / why we need it:

In k8s v1.16 some APIs are deprecated and removed.
https://kubernetes.io/blog/2019/07/18/api-deprecations-in-1-16/

As a result, installation failed. I migrate to use supported API versions.

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

fixes [stable/prometheus] deprecated APIs removed in k8s 1.16 #17267

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

DCO signed
Chart Version bumped
Variables are documented in the README.md
Title of the PR starts with chart name (e.g. [stable/chart])

k8s-ci-robot · 2019-09-19T23:00:06Z

Hi @oke-py. Thanks for your PR.

I'm waiting for a helm member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

oke-py · 2019-09-20T23:37:03Z

/assign @mgoodness

starkers · 2019-09-23T18:02:08Z

(Sadly..). This might be worth creating a new variable cause I suspect there are simply too many people on older k8s still.. Just throwing that in, I'm worried about a flood of PRs afterwards asking for "legacy support"

oke-py · 2019-09-23T22:41:55Z

@starkers All right. I'll try it.

oke-py · 2019-09-23T23:21:08Z

PodSecurityPolicy policy/v1beta1 API, available since v1.10.
DaemonSet, Deployment, StatefulSet, and ReplicaSet apps/v1 API, available since v1.9.
https://kubernetes.io/blog/2019/07/18/api-deprecations-in-1-16/
So it can support v.1.10 or later.

starkers · 2019-09-23T23:33:44Z

Ahh geat!

Sorry, I should have looked this up before FUD, those are ancient! Then this should be a really good fix

oke-py · 2019-09-23T23:41:17Z

@starkers I pushed update. PTAL

zaneclaes

Thanks @oke-py for jumping on this and @starkers for the review. I'd love to see this merged, since any of us who were livepatched into the stable kubernetes upgrade are now dead in the water with Prometheus. Changes lgtm.

zaneclaes · 2019-09-30T22:26:23Z

stable/prometheus/templates/_helpers.tpl

+{{- if semverCompare ">=1.3-0, <1.10-0" .Capabilities.KubeVersion.GitVersion -}}
+{{- print "extensions/v1beta1" -}}
+{{- else if semverCompare "^1.10-0" .Capabilities.KubeVersion.GitVersion -}}
+{{- print "policy/v1beta1 " -}}


Super nit-pick: potential extra space here (almost certainly harmless).

@zaneclaes I fixed it. thx

zanhsieh · 2019-10-03T01:36:08Z

/ok-to-test

WarheadsSE · 2019-10-03T21:03:05Z

stable/prometheus/Chart.yaml

@@ -1,6 +1,6 @@
 apiVersion: v1
 name: prometheus
-version: 9.1.1
+version: 9.1.2


With a change as complex as the apiVersion, and the introduction of the requirement for Tiller via the .Capabilities call, I feel this should be more than a patch version change.

@WarheadsSE I updated the version to 9.2.0.

Minor version upgrade seems right to me for this. I'm unclear on the tide check, but looks like we need the lgtm to come from a maintainer..?

@zaneclaes yes, it requires /lgtm comment from a maintainer.

WarheadsSE · 2019-10-03T21:22:08Z

Silliest question of the PR: how many people are actively using clusters before k8s 1.10?

oke-py · 2019-10-03T21:45:49Z

@WarheadsSE I'm not sure, but this PR also supports versions prior to 1.10.

WarheadsSE · 2019-10-04T16:57:04Z

@oke-py And I appreciate that 🙇‍♂️

TL;DR: TIL .Capabilities.KubeVersion doesn't break helm template, but is useless against newer versions of Kubernetes unless you pass the --kube-version

@starkers for the sake of my own clarity, can you explain (or point to doc/code) how .Capabilities.KubeVersion is populated when using helm template, thus without Tiller? I see pkg/chartutil/capabilties.go seems to set a defaults (for some time now). 2.9.0 through all of released 2.14.x set this as k8s 1.9.0. Only master has a newer version (v1.14.0) thanks to a change from 4 months ago.

The problem is then that even with a k8s cluster running 1.16.0, they will end up with objects not compatible when using Helm v2 in any released version.

$ asdf install helm 2.14.3 # packages with k8s 1.9.x "default"
$ asdf local helm 2.14.3
$ asdf current helm
helm           2.14.3    (set by /tmp/tmp.QWMXO4AqT4/charts/stable/prometheus/.tool-versions)
$ helm template . --set rbac.create=true,podSecurityPolicy.enabled=true | grep -B1 PodSecurityPolicy
apiVersion: extensions/v1beta1
kind: PodSecurityPolicy
--
...
$ helm template . --kube-version 1.16.0 --set rbac.create=true,podSecurityPolicy.enabled=true | grep -B1 PodSecurityPolicy
apiVersion: policy/v1beta1
kind: PodSecurityPolicy
--
...

starkers · 2019-10-04T18:54:04Z

Without tiller I'm actually not sure if or how Capabilities could be determined.. (I have personally moved on from helm).. I'm fairly sure though that It may be possible to override such templates (I've never tried but it supports ifs so surely we can add an actual variable)

On the other hand I feel that the majority of people are using tiller.. So the pragmatic thing should be to support this use-case first.. Mostly because this chart is currently broken for anyone with a "current" cluster..

For those who are templating... Perhaps there is a helm way of setting such a value so the template can be updated in one command, however with absolute certainty (because they're already having to maintain their own definition (I assume in git) it's just one sed command for them to fix right?

oke-py · 2019-10-05T04:41:41Z

it's better to avoid using Capabilities like this original commit?
41f3eaa

WarheadsSE · 2019-10-05T04:59:18Z

No, the use is viable here. It is just a note that those not using tiller must manually specify a version greater than 1.9.0 👍

k8s-ci-robot · 2019-10-19T09:52:14Z

@oke-py: GitHub didn't allow me to assign the following users: mgoodness.

Note that only helm members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @mgoodness
PTAL. Thanks in advance.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

…sions Signed-off-by: Naoki Oketani <okepy.naoki@gmail.com>

…lper.tpl Signed-off-by: Naoki Oketani <okepy.naoki@gmail.com>

…in helper.tpl Signed-off-by: Naoki Oketani <okepy.naoki@gmail.com>

Signed-off-by: Naoki Oketani <okepy.naoki@gmail.com>

oke-py · 2019-10-24T14:33:46Z

/assign @davidkarlsen
I think you are one of reviewers. Sorry if it's wrong.

bcg62 · 2019-10-27T15:50:07Z

anything still outstanding here to push this along? It's a show stopper for deployment on 1.16

oke-py · 2019-10-27T16:10:59Z

@davidkarlsen @mgoodness @gianrubio Could you please review and merge?

zanhsieh · 2019-10-27T17:12:45Z

@oke-py
@davidkarlsen is not one of this chart owners, but he is a helm member. Please search "OWNER" from k8s-ci-robot message and click the link above. We need to push this PR passed and elect another OWNERs to maintain this chart, since neither @mgoodness nor @gianrubio are willing to maintain this chart. See this message captured below in kubernetes slack channel:

---- Friday, October 18th ----
zanhsieh 10:08 PM
Hi Michael. Sorry for bothering you again. Would you mind to LGTM this PR please? #17979

goodness 10:09 PM
Hi Ming. afraid I haven’t been an active maintainer of the repo for a couple of years now. don’t have merge access

zanhsieh 10:10 PM
But you are still on the list of the owners. https://github.com/helm/charts/blob/master/stable/prometheus/OWNERS
I feel really really sorry for bothering you, but @gianrubio has lost interest in the prometheus helm chart.

oke-py · 2019-10-27T17:29:35Z

@zanhsieh

@davidkarlsen is not one of this chart owners, but he is a helm member.

Yes, I know, but I found he closed some PRs for stable/prometheus, and so I asked him to do the same for this PR.

Please search "OWNER" from k8s-ci-robot message and click the link above.

What link do you mean? helm members?

We need to push this PR passed and elect another OWNERs to maintain this chart

What should we do? I can be one of the maintainers?

zanhsieh · 2019-10-27T18:25:09Z

@oke-py

Please search "OWNER" from k8s-ci-robot message and click the link above.
What link do you mean? helm members?

This one: https://github.com/helm/charts/blob/master/stable/prometheus/OWNERS

What should we do? I can be one of the maintainers?

Yes you could, but you need to get this PR or have any previous PR passed in this chart, then open another modification PR to OWNERS file w/ adding yourself to it (of course chart version needs to get bump to latest). Finally asked a helm member to LGTM. See:

#13867

Once helm member received your PR for adding owners, he should invite you to helm chart org., then PR merged. After that you should be able to LGTM this chart PR.

oke-py · 2019-10-28T02:54:26Z

This one: https://github.com/helm/charts/blob/master/stable/prometheus/OWNERS

I've checked out this file.
In this PR, I couldn't get any response from them, I asked davidkarlsen to review.

So, who can review & merge this PR?

zanhsieh · 2019-10-28T08:50:28Z

@oke-py
I would check those recently close PR to see any helm member available. Any helm member should have permission to overwrite owner(s) of this chart. But you have to be politely and tell the situations, e.g. not intend to bother their own schedule / soliciting and promise will resume to normal once new owner(s) elected.

jsimomaa · 2019-11-04T13:24:46Z

Any updates on this?

tlemarchand · 2019-11-04T20:05:59Z

@gianrubio you are the latest addition to the reviewer team, can you review this long awaited PR ?
Thank you @oke-py for this work by the way !

maorfr · 2019-11-05T06:27:46Z

/lgtm

k8s-ci-robot · 2019-11-05T06:27:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maorfr, oke-py

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:

~~stable/prometheus/OWNERS~~ [maorfr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…sions (helm#17268) * [stable/prometheus] migrate API versions from deprecated, removed versions Signed-off-by: Naoki Oketani <okepy.naoki@gmail.com> * [stable/prometheus] set prometheus.podSecurityPolicy.apiVersion in helper.tpl Signed-off-by: Naoki Oketani <okepy.naoki@gmail.com> * [stable/prometheus] set prometheus.{deployment,daemonset}.apiVersion in helper.tpl Signed-off-by: Naoki Oketani <okepy.naoki@gmail.com> * trim whitespace Signed-off-by: Naoki Oketani <okepy.naoki@gmail.com> * [stable/prometheus] bump minor version Signed-off-by: Naoki Oketani <okepy.naoki@gmail.com>

helm-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 19, 2019

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 19, 2019

k8s-ci-robot requested review from gianrubio and mgoodness September 19, 2019 23:00

helm-bot added the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Sep 19, 2019

k8s-ci-robot assigned mgoodness Sep 20, 2019

helm-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 23, 2019

OJFord mentioned this pull request Sep 28, 2019

[helm] Deploying loki-stack with prometheus & grafana fails in k8s 1.16 grafana/loki#1080

Closed

zaneclaes approved these changes Sep 30, 2019

View reviewed changes

helm-bot removed the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Sep 30, 2019

helm-bot unassigned mgoodness Sep 30, 2019

helm-bot added the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Sep 30, 2019

k8s-ci-robot added ok-to-test and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 3, 2019

WarheadsSE reviewed Oct 3, 2019

View reviewed changes

oke-py mentioned this pull request Oct 19, 2019

[stable/prometheus] is not compatible with the changes in K8s v1.16 #18076

Closed

HassanHashemi mentioned this pull request Oct 21, 2019

Fix k8 1.16 breaking changes for Prometheus chart #17974

Closed

oke-py added 5 commits October 24, 2019 23:28

[stable/prometheus] migrate API versions from deprecated, removed ver…

022f987

…sions Signed-off-by: Naoki Oketani <okepy.naoki@gmail.com>

[stable/prometheus] set prometheus.podSecurityPolicy.apiVersion in he…

bc357e9

…lper.tpl Signed-off-by: Naoki Oketani <okepy.naoki@gmail.com>

[stable/prometheus] set prometheus.{deployment,daemonset}.apiVersion …

88924fe

…in helper.tpl Signed-off-by: Naoki Oketani <okepy.naoki@gmail.com>

trim whitespace

48ff0e9

Signed-off-by: Naoki Oketani <okepy.naoki@gmail.com>

[stable/prometheus] bump minor version

e377149

Signed-off-by: Naoki Oketani <okepy.naoki@gmail.com>

k8s-ci-robot assigned davidkarlsen Oct 24, 2019

avarf mentioned this pull request Oct 25, 2019

Prometheus is not compatible with the changes in K8s v1.16 prometheus/prometheus#6220

Closed

k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 5, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 5, 2019

k8s-ci-robot merged commit dac63a3 into helm:master Nov 5, 2019

This was referenced Nov 5, 2019

[stable/kube-registry-proxy] Update api versions to support k8s 1.16 #18047

Closed

[stable/kured] Change psp apiVersion for K8s 1.16 #18373

Merged

oke-py deleted the prometheus branch November 5, 2019 08:33

HassanHashemi mentioned this pull request Nov 7, 2019

[stable/prometheus] Chart incompatible with Tiller canary+unreleased #17735

Closed

[stable/prometheus] migrate API versions from deprecated, removed versions #17268

[stable/prometheus] migrate API versions from deprecated, removed versions #17268

Conversation

oke-py commented Sep 19, 2019

Is this a new chart

What this PR does / why we need it:

Which issue this PR fixes

Special notes for your reviewer:

Checklist

k8s-ci-robot commented Sep 19, 2019

oke-py commented Sep 20, 2019

starkers commented Sep 23, 2019

oke-py commented Sep 23, 2019

oke-py commented Sep 23, 2019

starkers commented Sep 23, 2019

oke-py commented Sep 23, 2019

zaneclaes left a comment

Choose a reason for hiding this comment

zaneclaes Sep 30, 2019

Choose a reason for hiding this comment

oke-py Sep 30, 2019

Choose a reason for hiding this comment

zanhsieh commented Oct 3, 2019

WarheadsSE Oct 3, 2019

Choose a reason for hiding this comment

oke-py Oct 3, 2019

Choose a reason for hiding this comment

oke-py Oct 4, 2019

Choose a reason for hiding this comment

zaneclaes Oct 4, 2019

Choose a reason for hiding this comment

oke-py Oct 5, 2019

Choose a reason for hiding this comment

WarheadsSE commented Oct 3, 2019

oke-py commented Oct 3, 2019

WarheadsSE commented Oct 4, 2019

starkers commented Oct 4, 2019

oke-py commented Oct 5, 2019

WarheadsSE commented Oct 5, 2019 via email

k8s-ci-robot commented Oct 19, 2019

oke-py commented Oct 24, 2019

bcg62 commented Oct 27, 2019

oke-py commented Oct 27, 2019

zanhsieh commented Oct 27, 2019

oke-py commented Oct 27, 2019

zanhsieh commented Oct 27, 2019 • edited Loading

oke-py commented Oct 28, 2019

zanhsieh commented Oct 28, 2019 • edited Loading

jsimomaa commented Nov 4, 2019

tlemarchand commented Nov 4, 2019

maorfr commented Nov 5, 2019

k8s-ci-robot commented Nov 5, 2019

zanhsieh commented Oct 27, 2019 •

edited

Loading

zanhsieh commented Oct 28, 2019 •

edited

Loading