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

[doc] 6.1.3 Installation - add version to helm chart deployments #621

Closed
Martin-Weiss opened this issue Dec 3, 2019 · 14 comments
Closed
Assignees
Labels
AdminGuide Fix will change the Admin Guide Blocked Blocked by lack of information or external factors EngineeringInput NeedsEngineering Input Enhancement Fix is an enhancement PO/PM Product Owner / Product Manager needs to provide input
Milestone

Comments

@Martin-Weiss
Copy link
Contributor

6.1.3 Installation

https://documentation.suse.com/suse-caasp/4.0/html/caasp-admin/_monitoring.html#_installation

As there can be multiple helm chart versions available and not all versions might be compatible with all k8s versions - we should add the --version parameter to deploy the tested and supported helm chart version for a given CaaSP version. (instead of using the default "latest".

--> could you add the --version 9.2.0 for prometheus, --version 0.28.4 for nginx-ingress and so on?

@r0ckarong
Copy link
Contributor

@jordimassaguerpla I am trying to find a scenario where this becomes a problem. Each release has one specific k8s version and my expectation is that the "latest" image has been tested in that release.

@Martin-Weiss What does this scenario look like where you have multiple k8s versions in one cluster or install an older version of a chart? From what I understand, if we have a limitation on what chart needs to be installed this is specifically mentioned otherwise "latest" (at time of release) is tested and works.

@r0ckarong r0ckarong added AdminGuide Fix will change the Admin Guide Enhancement Fix is an enhancement Reported labels Dec 3, 2019
@r0ckarong r0ckarong added this to the Sprint 19 milestone Dec 3, 2019
@r0ckarong r0ckarong self-assigned this Dec 3, 2019
@Martin-Weiss
Copy link
Contributor Author

@Martin-Weiss What does this scenario look like where you have multiple k8s versions in one cluster or install an older version of a chart? From what I understand, if we have a limitation on what chart needs to be installed this is specifically mentioned otherwise "latest" (at time of release) is tested and works.

Extreme example - we release k8s 2.6 with caasp 7.3 and with that we release prometheus 47.3 as "latest". Now someone reads the documentation for his caasp v4.0.3 and wants to deploy prometheus. Does the latest version of prometheus run on k8s 1.15?

Btw. also the customized "values.yaml" files need to be versioned as depending on future changes the current prepared and adjusted values.yaml for i.e. prometheus 9.2.0 might not work with prometheus 17.7 ..

IMO we should always add --version for the helm chart we tested in QA to ensure end to end quality assurance.

@r0ckarong
Copy link
Contributor

r0ckarong commented Dec 3, 2019

The scenario laid out here is very unlikely and not supported at all but I can understand what your point is. There is a reasonably likely scenario where the latest chart won't work with something that is still interlocked in docs support.

@pgonin @thehejik @kravciak @atighineanu @fgerling Does QA test with specific versions per release or use "latest" at the time of testing?
@jordimassaguerpla Can we create this version interlock with a reasonably amount of effort?

The tricky bit here is to maintain the list of tested upstream images. The SUSE images we export to the generated images file via skuba.

@Martin-Weiss
Copy link
Contributor Author

Please keep in mind that the whole container story is about building "immutable" and "repeatable" setups and delivery from development down to the end user.

So using "latest" is in general should be counted a bad practice because this automatically "kills" the "repeatable".
How does someone know which version he was using and testing if "latest" is used and "latest" is changed by someone else?

@thehejik
Copy link
Contributor

thehejik commented Dec 3, 2019

@Martin-Weiss IMO we should be responsible only for official suse helm charts using tagged container images from registry.suse.com. We are not tied to :latest images but we are using specific versions as you can see below.

If you open for eg. https://github.com/SUSE/kubernetes-charts-suse-com/blob/master/stable/nginx-ingress/values.yaml you can see that the nginx-ingress is tied to specific versions of images which should be compatible with CaaSP v4 (not sure if we support all the versions - v3, v4 4.0.3 with k8s 1.15.2 and v4 4.1.0 with k8s 1.16.x)

The versioning would give sense for upstream helm charts but not for the ones officially supported by SUSE.

To answer your question: We are not using :latest but the version provided by suse helm chart.

@Martin-Weiss
Copy link
Contributor Author

@Martin-Weiss IMO we should be responsible only for official suse helm charts using tagged container images from registry.suse.com. We are not tied to :latest images but we are using specific versions as you can see below.

If you open for eg. https://github.com/SUSE/kubernetes-charts-suse-com/blob/master/stable/nginx-ingress/values.yaml you can see that the nginx-ingress is tied to specific versions of images which should be compatible with CaaSP v4 (not sure if we support all the versions - v3, v4 4.0.3 with k8s 1.15.2 and v4 4.1.0 with k8s 1.16.x)

The dependency helm-chart <-> image is fine as long as the version is specified instead of "no tag" or "no version" in the helm chart.

The versioning would give sense for upstream helm charts but not for the ones officially supported by SUSE.

In case SUSE is going to change, add or remove variables in helm charts we will have the same problem like what we have upstream already where each helm chart uses its one naming conventions for variables and adds/removes/changes them more or less randomly ;-).

Or do we guarantee that the variables defined and used stay consistent and will never change?

To answer your question: We are not using :latest but the version provided by suse helm chart.

During helm install - you specify --version ? Otherwise you use the "latest" helm chart. (keep in mind that I am not talking about image versions - I am talking about helm chart versions).

@thehejik
Copy link
Contributor

thehejik commented Dec 3, 2019

Well I'm not aware that our suse helm chart repo is supporting any kind of versioning - there is just one "stable" version AFAIK.

@Martin-Weiss
Copy link
Contributor Author

Well I'm not aware that our suse helm chart repo is supporting any kind of versioning - there is just one "stable" version AFAIK.

Just do "helm search" and you will see.. the helm chart repo is just an "index.json with some -.tar.gz"

Just try

helm search suse -l and you will see that there are multiple versions of charts available - as an example:

suse/nginx-ingress              0.28.4                  0.15.0          An nginx Ingress controller that uses ConfigMap...
suse/nginx-ingress              0.28.3                  0.15.0          An nginx Ingress controller that uses ConfigMap...

@r0ckarong
Copy link
Contributor

cc @kkaempf

@r0ckarong r0ckarong added the PO/PM Product Owner / Product Manager needs to provide input label Dec 3, 2019
@jordimassaguerpla
Copy link
Member

I think it makes sense that we write down the version of the helm chart we have tested. We already do that with k8s and cilium, so we could have a variable for that and use that in the document, or?

https://github.com/SUSE/doc-caasp/blob/master/adoc/attributes.adoc#L23

Am I missing something here that makes this complicated?

@jordimassaguerpla
Copy link
Member

Either that, or we add the helm versions in an external .txt file, like we do for the images:

https://documentation.suse.com/external-tree/en-us/suse-caasp/4/skuba-cluster-images.txt

@r0ckarong
Copy link
Contributor

I think it makes sense that we write down the version of the helm chart we have tested. We already do that with k8s and cilium, so we could have a variable for that and use that in the document, or?

https://github.com/SUSE/doc-caasp/blob/master/adoc/attributes.adoc#L23

Am I missing something here that makes this complicated?

The one thing that comes to mind immediately is that when we need to ship a new chart or image version to do a fix after QA on the initial release, the docs will be wrong. This adds more complexity to get the release right the first time.

@jordimassaguerpla
Copy link
Member

What about generating a txt file or appending it to the skuba-cluster-images.txt ? So the docs will be fine

@r0ckarong r0ckarong modified the milestones: Sprint 19, Sprint 20 Dec 9, 2019
@r0ckarong r0ckarong added the Blocked Blocked by lack of information or external factors label Jan 13, 2020
@r0ckarong r0ckarong modified the milestones: Sprint 20, Sprint 21 Jan 13, 2020
@r0ckarong r0ckarong removed the Blocked Blocked by lack of information or external factors label Jan 21, 2020
@r0ckarong r0ckarong added the Blocked Blocked by lack of information or external factors label Jan 27, 2020
@r0ckarong r0ckarong removed this from the Sprint 22 milestone Feb 24, 2020
@r0ckarong
Copy link
Contributor

The indiividual open problems have follow up issues. I'll close this discussion and we'll work on the other requests.

@r0ckarong r0ckarong added this to the Sprint 40 milestone Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AdminGuide Fix will change the Admin Guide Blocked Blocked by lack of information or external factors EngineeringInput NeedsEngineering Input Enhancement Fix is an enhancement PO/PM Product Owner / Product Manager needs to provide input
Projects
None yet
Development

No branches or pull requests

4 participants