Remove namespace from linkerd-jaeger chart #6669
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Another part of #6584 based off of #6635 (alpeb/no-ns-helm-core) Stop rendering `namespace.yaml` in the `linkerd-jaeger` chart, same as we did in #6635. This makes use of a Helm `post-install` hook to add the `linkerd.io/extension: jaeger` label to the namespace created by Helm. The `linkerd.io/inject: enabled` and `config.linkerd.io/proxy-await: enabled` annotations have been moved down from the namespace and into each workload.
Codecov Report
@@ Coverage Diff @@
## alpeb/no-ns-helm-core #6669 +/- ##
========================================================
Coverage ? 45.38%
========================================================
Files ? 182
Lines ? 24273
Branches ? 290
========================================================
Hits ? 11016
Misses ? 12484
Partials ? 773
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
adleong
approved these changes
Aug 13, 2021
Pothulapati
approved these changes
Aug 16, 2021
. Works as expected!
alpeb
added a commit
that referenced
this pull request
Dec 10, 2021
…inkerd-control-plane` (#6635) Fixes #6584 #6620 #7405 # Namespace Removal With this change, the `namespace.yaml` template is rendered only for CLI installs and not Helm, and likewise the `namespace:` entry in the namespace-level objects (using a new `partials.namespace` helper). The `installNamespace` and `namespace` entries in `values.yaml` have been removed. There in the templates where the namespace is required, we moved from `.Values.namespace` to `.Release.Namespace` which is filled-in automatically by Helm. For the CLI, `install.go` now explicitly defines the contents of the `Release` map alongside `Values`. The proxy-injector has a new `linkerd-namespace` argument given the namespace is no longer persisted in the `linkerd-config` ConfigMap, so it has to be passed in. To pass it further down to `injector.Inject()` without modifying the `Handler` signature, a closure was used. ------------ Update: Merged-in #6638: Similar changes for the `linkerd-viz` chart: Stop rendering `namespace.yaml` in the `linkerd-viz` chart. The additional change here is the addition of the `namespace-metadata.yaml` template (and its RBAC), _not_ rendered in CLI installs, which is a Helm `post-install` hook, consisting on a Job that executes a script adding the required annotations and labels to the viz namespace using a PATCH request against kube-api. The script first checks if the namespace doesn't already have an annotations/labels entries, in which case it has to add extra ops in that patch. --------- Update: Merged-in the approved #6643, #6665 and #6669 which address the `linkerd2-cni`, `linkerd-multicluster` and `linkerd-jaeger` charts. Additional changes from what's already mentioned above: - Removes the install-namespace option from `linkerd install-cni`, which isn't found in `linkerd install` nor `linkerd viz install` anyways, and it would add some complexity to support. - Added a dependency on the `partials` chart to the `linkerd-multicluster-link` chart, so that we can tap on the `partials.namespace` helper. - We don't have any more the restriction on having the muticluster objects live in a separate namespace than linkerd. It's still good practice, and that's the default for the CLI install, but I removed that validation. Finally, as a side-effect, the `linkerd mc allow` subcommand was fixed; it has been broken for a while apparently: ```console $ linkerd mc allow --service-account-name foobar Error: template: linkerd-multicluster/templates/remote-access-service-mirror-rbac.yaml:16:7: executing "linkerd-multicluster/templates/remote-access-service-mirror-rbac.yaml" at <include "partials.annotations.created-by" $>: error calling include: template: no template "partials.annotations.created-by" associated with template "gotpl" ``` --------- Update: see helm/helm#5465 describing the current best-practice # Core Helm Charts Split This removes the `linkerd2` chart, and replaces it with the `linkerd-crds` and `linkerd-control-plane` charts. Note that the viz and other extension charts are not concerned by this change. Also note the original `values.yaml` file has been split into both charts accordingly. ### UX ```console $ helm install linkerd-crds --namespace linkerd --create-namespace linkerd/linkerd-crds ... # certs.yaml should contain identityTrustAnchorsPEM and the identity issuer values $ helm install linkerd-control-plane --namespace linkerd -f certs.yaml linkerd/linkerd-control-plane ``` ### Upgrade As explained in #6635, this is a breaking change. Users will have to uninstall the `linkerd2` chart and install these two, and eventually rollout the proxies (they should continue to work during the transition anyway). ### CLI The CLI install/upgrade code was updated to be able to pick the templates from these new charts, but the CLI UX remains identical as before. ### Other changes - The `linkerd-crds` and `linkerd-control-plane` charts now carry a version scheme independent of linkerd's own versioning, as explained in #7405. - These charts are Helm v3, which is reflected in the `Chart.yaml` entries and in the removal of the `requirements.yaml` files. - In the integration tests, replaced the `helm-chart` arg with `helm-charts` containing the path `./charts`, used to build the paths for both charts. ### Followups - Now it's possible to add a `ServiceProfile` instance for Destination in the `linkerd-control-plane` chart.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Another part of #6584 based off of #6635 (alpeb/no-ns-helm-core)
Stop rendering
namespace.yamlin thelinkerd-jaegerchart, same as we did in #6635.This makes use of a Helm
post-installhook to add thelinkerd.io/extension: jaegerlabel to the namespace created by Helm.The
linkerd.io/inject: enabledandconfig.linkerd.io/proxy-await: enabledannotations have been moved down from the namespace and into each workload.