Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

[stable/spinnaker] Move default profile settings from values to the template #13918

Merged
merged 3 commits into from
May 17, 2019

Conversation

legal90
Copy link
Contributor

@legal90 legal90 commented May 16, 2019

What this PR does / why we need it:

Now templates/configmap/additional-profile-configmaps.yaml template has advanced logic with conditions and dict merging, which brings us the following:

1. Move default profile settings from values to the template.

Default values for gate-local.yml profile (introduced in #10771) will be injected automatically if gce or alb ingress controller is used.
So, there will be no need for some users to set halyard.additionalProfileConfigMaps to true to get it working. Also, other users won't have to explicitly unset halyard.additionalProfileConfigMaps.data.gate-local\.yml if they don't use gce or alb ingress.

2. Disable S3 versioning if Minio is used

Minio is the local S3-compatible storage engine, which is enabled in stable/spinnaker by default (as an alternative to AWS S3). However, it doesn't support versioning, which makes the front50 microservice failing to start.
So the versioning should be disabled according to Spinnaker docs:
https://www.spinnaker.io/setup/install/storage/minio/#editing-your-storage-settings

3. Changes in halyard.additionalProfileConfigMaps.data map

  • I removed halyard.additionalProfileConfigMaps.create key from values.yaml because now this ConfigMap will always be created and mounted, in order to get the default settings (p.1) available.
  • I also changed the expected type of values in halyard.additionalProfileConfigMaps.data map. Now it could be not only a string, but a dict (map[string]interface{}) too, so users can override any of the fields defined by default, for example:
# overrides-values.yml
halyard:
  additionalProfileConfigMaps:
    data:
      front50-local.yml:  # it's a map, no "|-" here
        foo: bar
        spinnaker:
          s3:
            endpoint: OVERRIDEN_VALUE

will lead to this content of the ConfigMap:

apiVersion: v1
kind: ConfigMap
metadata:
  # ...
data:
  front50-local.yml: |
    foo: bar
    spinnaker:
      s3:
        endpoint: OVERRIDEN_VALUE
        versioning: false

Special notes for your reviewer:

Despite all the changes, this PR saves full backward compatibility. If the user has set halyard.additionalProfileConfigMaps.create to true and defined any profiles in halyard.additionalProfileConfigMaps.data, these profiles will be always respected instead of the defaults generated by the template conditions (if they overlap).

Checklist

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

  • DCO signed
  • Chart Version bumped
  • title of the PR contains starts with chart name e.g. [stable/chart]

@helm-bot helm-bot added Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 16, 2019
@k8s-ci-robot 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 May 16, 2019
@k8s-ci-robot
Copy link
Contributor

Hi @legal90. 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.

@legal90 legal90 changed the title Spinnaker render profile cm [stable/spinnaker] Move default profile settings from values to the template May 16, 2019
@dwardu89
Copy link
Collaborator

/ok-to-test

@k8s-ci-robot 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 May 16, 2019
…emplate

Add support of passing actual blocks (aka `map[string]interface{}`) to `halyard.additionalProfileConfigMaps.data`,
which allows to override any of the default field as blocks are deeply merged.

These changes also keep the full backward compatibility with the old way passing data to `halyard.additionalProfileConfigMaps.data`.
Users can still pass the string values there and they will be applied only if `halyard.additionalProfileConfigMaps.create` is set to `true`.

Also, `halyard.additionalProfileConfigMaps.create` has been removed from the default values
because now this ConfigMap will always be created.

Signed-off-by: Mikhail Zholobov <legal90@gmail.com>
Minio doesn't support versioning, so we should disable it on the Front50 side in this case.
https://www.spinnaker.io/setup/install/storage/minio/#editing-your-storage-settings

Otherwise, "pull-charts-e2e" tests will always fail because of Front50 errors related to
the versioning on S3-like storage backend.

Signed-off-by: Mikhail Zholobov <legal90@gmail.com>
@legal90 legal90 force-pushed the spinnaker-render-profile-cm branch from 786dd55 to fd15834 Compare May 16, 2019 19:31
Signed-off-by: Mikhail Zholobov <legal90@gmail.com>
@legal90 legal90 force-pushed the spinnaker-render-profile-cm branch from fd15834 to c82dd65 Compare May 16, 2019 19:42
@dwardu89
Copy link
Collaborator

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 17, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dwardu89, legal90

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 17, 2019
@k8s-ci-robot k8s-ci-robot merged commit 31ad12e into helm:master May 17, 2019
amine7536 pushed a commit to amine7536/charts that referenced this pull request May 21, 2019
…emplate (helm#13918)

* [stable/spinnaker] Move default profile settings from values to the template

Add support of passing actual blocks (aka `map[string]interface{}`) to `halyard.additionalProfileConfigMaps.data`,
which allows to override any of the default field as blocks are deeply merged.

These changes also keep the full backward compatibility with the old way passing data to `halyard.additionalProfileConfigMaps.data`.
Users can still pass the string values there and they will be applied only if `halyard.additionalProfileConfigMaps.create` is set to `true`.

Also, `halyard.additionalProfileConfigMaps.create` has been removed from the default values
because now this ConfigMap will always be created.

Signed-off-by: Mikhail Zholobov <legal90@gmail.com>

* [stable/spinnaker] Disable S3 versioning if Minio is enabled

Minio doesn't support versioning, so we should disable it on the Front50 side in this case.
https://www.spinnaker.io/setup/install/storage/minio/#editing-your-storage-settings

Otherwise, "pull-charts-e2e" tests will always fail because of Front50 errors related to
the versioning on S3-like storage backend.

Signed-off-by: Mikhail Zholobov <legal90@gmail.com>

* [stable/spinnaker] Bump chart version

Signed-off-by: Mikhail Zholobov <legal90@gmail.com>
eyenx pushed a commit to eyenx/charts that referenced this pull request May 28, 2019
…emplate (helm#13918)

* [stable/spinnaker] Move default profile settings from values to the template

Add support of passing actual blocks (aka `map[string]interface{}`) to `halyard.additionalProfileConfigMaps.data`,
which allows to override any of the default field as blocks are deeply merged.

These changes also keep the full backward compatibility with the old way passing data to `halyard.additionalProfileConfigMaps.data`.
Users can still pass the string values there and they will be applied only if `halyard.additionalProfileConfigMaps.create` is set to `true`.

Also, `halyard.additionalProfileConfigMaps.create` has been removed from the default values
because now this ConfigMap will always be created.

Signed-off-by: Mikhail Zholobov <legal90@gmail.com>

* [stable/spinnaker] Disable S3 versioning if Minio is enabled

Minio doesn't support versioning, so we should disable it on the Front50 side in this case.
https://www.spinnaker.io/setup/install/storage/minio/#editing-your-storage-settings

Otherwise, "pull-charts-e2e" tests will always fail because of Front50 errors related to
the versioning on S3-like storage backend.

Signed-off-by: Mikhail Zholobov <legal90@gmail.com>

* [stable/spinnaker] Bump chart version

Signed-off-by: Mikhail Zholobov <legal90@gmail.com>
@legal90 legal90 deleted the spinnaker-render-profile-cm branch May 28, 2019 17:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). lgtm Indicates that a PR is ready to be merged. ok-to-test size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants