Skip to content

Handle resource apiVersion upgrades#7625

Closed
jlegrone wants to merge 1 commit intohelm:masterfrom
DataDog:jlegrone/no-error-same-gvk
Closed

Handle resource apiVersion upgrades#7625
jlegrone wants to merge 1 commit intohelm:masterfrom
DataDog:jlegrone/no-error-same-gvk

Conversation

@jlegrone
Copy link
Copy Markdown
Member

@jlegrone jlegrone commented Feb 18, 2020

Related to #6850, and an alternative implementation to #7575.

Steps to Reproduce

Create a chart with

apiVersion: apps/v1beta2
kind: Deployment

Deploy a release of that chart, then switch to apps/v1:

- apiVersion: apps/v1beta2
+ apiVersion: apps/v1
kind: Deployment

Run upgrade, and you should get the following error before this patch:

rendered manifests contain a new resource that already exists. Unable to continue with update: existing resource conflict: namespace: default, name: version-migrate-test, existing_kind: apps/v1, Kind=Deployment, new_kind: apps/v1, Kind=Deployment

@jlegrone
fix: handle resource apiVersion upgrades
d8bff5f 
Signed-off-by: Jacob LeGrone <git@jacob.work>
@helm-bot helm-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 18, 2020
@bacongobbler
Copy link
Copy Markdown
Member

bacongobbler commented Feb 18, 2020

thanks @jlegrone! Will give this a spin later this afternoon.

@bacongobbler
Copy link
Copy Markdown
Member

bacongobbler commented Feb 18, 2020

First off: this is MUCH cleaner of an implementation. Thank you.

Here's what I tested:

  1. I installed a sample chart with a Deployment using the extensions/v1beta1 API
  • upgrade without changing the apiVersion - ✔️
  • switch template to use the apps/v1 API and upgrade - ✔️
  1. I installed a sample chart with a Deployment using the apps/v1 API
  • upgrade without changing the apiVersion - ✔️
  1. install a sample chart with a Deployment that already exists in the cluster

In case 3, I found an issue with this PR's behaviour.

Helm 3.1.0:

><> helm template wordpress wordpress/ --show-only templates/deployment.yaml | kubectl apply -f -
deployment.apps/wordpress configured
><> helm install wordpress wordpress/
Error: rendered manifests contain a resource that already exists. Unable to continue with install: existing resource conflict: namespace: default, name: wordpress, existing_kind: apps/v1, Kind=Deployment, new_kind: apps/v1, Kind=Deployment
><> helm list
NAME	NAMESPACE	REVISION	UPDATED	STATUS	CHART	APP VERSION

existingResourceConflict returns an error, preventing the release from being created.

With this PR:

><> ~/bin/helm version
version.BuildInfo{Version:"v3.1+unreleased", GitCommit:"d8bff5fdbd57234ad8a78c91a6688d3ff41a0cd4", GitTreeState:"clean", GoVersion:"go1.13.8"}
><> ~/bin/helm install wordpress ./wordpress/
Error: deployments.apps "wordpress" already exists
><> ~/bin/helm list
NAME     	NAMESPACE	REVISION	UPDATED                               	STATUS	CHART          	APP VERSION
wordpress	default  	1       	2020-02-18 15:32:56.23260213 -0800 PST	failed	wordpress-7.5.4	5.2.4

A failed pre-flight check should not deploy a release in a failed state. Would you mind looking further into this?

@jlegrone jlegrone added the wip work in progress label Feb 19, 2020
@hickeyma
Copy link
Copy Markdown
Contributor

hickeyma commented Feb 19, 2020

Thanks @jlegrone for this PR. It seems like a clean way to handle the API deprecation issue.

I did some testing similar to @bacongobbler but unfortunately didn't have the same success. My angle of testing is from the #7219 view point.

I tested as follows:

  1. Installed scaffold chart in k8s 1.15 cluster (using helm from master branch) with following apiVersion:
# Source: chrt-7625/templates/deployment.yaml
apiVersion: extensions/v1beta1

$ helm ls
NAME         	NAMESPACE	REVISION	UPDATED                                	STATUS  	CHART             	APP VERSION
chrt-7625    	default  	1       	2020-02-19 14:51:41.390839657 +0000 UTC	deployed	chrt-7625-0.1.0   	1.0

  1. Upgraded the cluster to 1.16 including the worker node

  2. Built Helm binary with this PR changes

  3. Tried but failed to upgrade the release chrt-7625 as follows:

  • apiVersion: extensions/v1beta1- FAILED
  • apiVersion: apps/v1 - FAILED
$ helm upgrade chrt-7625 chrt-7625/
Error: UPGRADE FAILED: unable to build kubernetes objects from current release manifest: unable to recognize "": no matches for kind "Deployment" in version "extensions/v1beta1"
  1. Succeeded in new install of chart chrt-7625 with apiVersion: apps/v1 in deployment

@bacongobbler bacongobbler mentioned this pull request Feb 19, 2020
Closed
@hickeyma
Copy link
Copy Markdown
Contributor

hickeyma commented Feb 19, 2020

Talking to @bacongobbler on Slack confirms that my testing is more concerned with the upgrading the cluster which automatically updates the resources APIs (to the latest version) in the cluster, which are then at odds with the API versions in the Helm release metadata. This is the issue #7219 .

@jlegrone
Copy link
Copy Markdown
Member Author

jlegrone commented Feb 20, 2020

Still working on this, unfortunately it's more involved than it appeared at first glance.

@jlegrone jlegrone mentioned this pull request Feb 21, 2020
Merged
@jlegrone
Copy link
Copy Markdown
Member Author

jlegrone commented Feb 21, 2020

Superseded by #7649

@jlegrone jlegrone closed this Feb 21, 2020
@jlegrone jlegrone deleted the jlegrone/no-error-same-gvk branch February 21, 2020 02:15
iaguis added a commit to kinvolk/lokomotive that referenced this pull request Jun 3, 2020
@iaguis
components: add required labels and annotations for Helm updates
23f29c5 
When upgrading a component that had an apiVersion change, Helm would
complain with "rendered manifests contain a new resource that already
exists".

This was fixed in Helm 3.2.0 by adding labels and annotations to mark a
resource as managed by Helm.

This adds those labels and annotations to problematic objects in
Lokomotive components.

We'll probably need to add them to more objects as apiVersions change,
since we'll check upgrades when releasing new versions, we'll handle
those as they happen.

See helm/helm#7625 and
helm/helm#7649.
iaguis added a commit to kinvolk/lokomotive that referenced this pull request Jun 3, 2020
@iaguis
components: add required labels and annotations for Helm updates
90dca9c 
When upgrading a component that had an apiVersion change, Helm would
complain with "rendered manifests contain a new resource that already
exists".

This was fixed in Helm 3.2.0 by adding labels and annotations to mark a
resource as managed by Helm.

This adds those labels and annotations to problematic objects in
Lokomotive components.

We'll probably need to add them to more objects as apiVersions change,
since we'll check upgrades when releasing new versions, we'll handle
those as they happen.

See helm/helm#7625 and
helm/helm#7649.
iaguis added a commit to kinvolk/lokomotive that referenced this pull request Jun 3, 2020
@iaguis
components: add required labels and annotations for Helm updates
c8aa769 
When upgrading a component that had an apiVersion change, Helm would
complain with "rendered manifests contain a new resource that already
exists".

This was fixed in Helm 3.2.0 by adding labels and annotations to mark a
resource as managed by Helm.

This adds those labels and annotations to problematic objects in
Lokomotive components.

We'll probably need to add them to more objects as apiVersions change,
since we'll check upgrades when releasing new versions, we'll handle
those as they happen.

See helm/helm#7625 and
helm/helm#7649.
iaguis added a commit to kinvolk/lokomotive that referenced this pull request Jun 3, 2020
@iaguis
components: add required labels and annotations for Helm updates
df20cdb 
When upgrading a component that had an apiVersion change, Helm would
complain with "rendered manifests contain a new resource that already
exists".

This was fixed in Helm 3.2.0 by adding labels and annotations to mark a
resource as managed by Helm.

This adds those labels and annotations to problematic objects in
Lokomotive components.

We'll probably need to add them to more objects as apiVersions change,
since we'll check upgrades when releasing new versions, we'll handle
those as they happen.

See helm/helm#7625 and
helm/helm#7649.
iaguis added a commit to kinvolk/lokomotive that referenced this pull request Jun 5, 2020
@iaguis
components: add required labels and annotations for Helm updates
b019981 
When upgrading a component that had an apiVersion change, Helm would
complain with "rendered manifests contain a new resource that already
exists".

This was fixed in Helm 3.2.0 by adding labels and annotations to mark a
resource as managed by Helm.

This adds those labels and annotations to problematic objects in
Lokomotive components.

We'll probably need to add them to more objects as apiVersions change,
since we'll check upgrades when releasing new versions, we'll handle
those as they happen.

See helm/helm#7625 and
helm/helm#7649.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bacongobbler bacongobbler Awaiting requested review from bacongobbler

Assignees

No one assigned

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files. wip work in progress

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jlegrone @bacongobbler @hickeyma @helm-bot