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

[stable/spinnaker] Leverage Halyard for installation #6407

Merged
merged 63 commits into from
Aug 23, 2018

Conversation

viglesiasce
Copy link
Contributor

This pull requests removes most of the installation logic from the chart and instead leverages Halyard, the Spinnaker project's tool for creating, configuring, and managing Spinnaker deployments.

Benefits:

  1. Much easier customization
  2. Less burden to update Spinnaker version (one liner)
  3. Spinnaker Kubernetes v2 provider support
  4. Decouples Jenkins installation
  5. 2000 less lines of code to maintain

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 29, 2018
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 29, 2018
command:
- bash
- -c
- "cp /opt/halyard/config/* /tmp/config && printf 'server:\n address: 0.0.0.0\n' > /tmp/config/halyard-local.yml"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

server.address: 0.0.0.0\n might be a little cleaner

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Good call.

Copy link
Collaborator

@lwander lwander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@timstoop
Copy link
Contributor

timstoop commented Jul 2, 2018

Nice, I like how you remove 10 times more lines than you add!

@viglesiasce
Copy link
Contributor Author

ha! Thanks @timstoop!! Going to add one more thing to make RBAC easier...

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 2, 2018
Copy link

@eaceaser eaceaser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/kubernetes/charts/pull/6407/files#diff-13a5d824f78f72778667d929a7c83c4dR64
https://github.com/kubernetes/charts/pull/6407/files#diff-61d64636826856804e1519cd255d54e8R2

Unless there are some changes in store to have this chart also deploy redis for caching at some point, I think we can remove the redis values and requirement as part of this too!

https://github.com/kubernetes/charts/pull/6407/files#diff-91f0ff1f45e99083851f8f4f1b909dbbR9

And the README copy would be outdated too I think

@@ -71,25 +51,9 @@ deck:
# hosts:
# - domain.com

gate:
allowedOriginsPattern: '^https?://(?:localhost|127.0.0.1|[^/]+\.example\.com)(?::[1-9]\d*)?/?$'

# Bucket to use when storing config data in S3 compatible storage
storageBucket: spinnaker
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be clearer to denormalize (for lack of a better term) storageBucket into the various storage providers' config blocks? Unless it gets used somewhere that I'm not seeing it seems like it should be configured alongside the storage provider, rather than outside of it as a top level key.

Vic Iglesias added 2 commits August 23, 2018 10:28
Copy link
Member

@scottrigby scottrigby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re #6407 (comment) and #6407 (comment), it seems to me the RBAC follows the best practices guidelines. rbac.create is configurable, as is serviceAccount.create. The service account names (serviceAccount.halyardName and serviceAccount.spinnakerName are also configurable, and a comment indicates that if left blank it's auto-generated from the release full name). The Spinnaker service account hard-codes default with this note:

Clouddriver does not currently allow config of its service account.

Personally I would say this looks good to merge. The MAJOR version is bumped, and it's fully up to date (a moment ago recently merged in updates from master).

@viglesiasce One request though, if there's any suggestions you have to help make https://github.com/helm/helm/blob/master/docs/chart_best_practices/rbac.md more clear, that would help #3011. Anyway thanks this looks 💯 to me

@scottrigby
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 23, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: scottrigby, viglesiasce

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:
  • OWNERS [scottrigby,viglesiasce]

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 merged commit 7113759 into helm:master Aug 23, 2018
ishabalin pushed a commit to NinthDecimal/charts that referenced this pull request Sep 3, 2018
* Use Halyard for Spinnaker installation

* Bump spinnaker version in Chart.yaml

* Add whitespace in helpers

* Remove dependency on Jenkins

* Fix cleanup

* Fix NOTES command for exec into hal

* No longer need override_config_map helper

* Simplify cleanup job

* Remove halyard-pvc empty file

* Update README

* Add lwander as maintainer of Spinnaker chart

* Make halyard-local config simpler

* Remove type: Opaque from kubeconfig map

* Mount GCS key on Halyard statefulset

* Fix credentials when using kubeConfig.enabled

* Allow custom config to be provided at install time.

* Fix port-forward commands in README

* Configure Halyard's RBAC

* Use redis deployed by helm

* Use Deck to route /gate requests

* Fix RBAC in default configuration

* Scope template helper names

* Helper for resource metadata

* Enable spinnakerFeatureFlags

* Fix deck service name

* Basic S3 support.

setting access keys is optional, if they are unset, the implicit
credentials from your environment will be used.

* Smallish updates.

* Runs the install script with bash -xe for log output.
* Makes the install script somewhat idempotent in the case of chart
  upgrades on an existing halyard config state.
* Fully qualifies serviceaccount names.
* Whitespace fixes.

* Enable 'jobs' feature flag by default

* Fix typo in README

* Follow RBAC best practices for chart

* Remove trailing space

* update helm dependencies

Update minio and redis deps to be their current releases

* support username/password for docker registry

* fix variable condition for registrysecret

* remove rogue sleep command from debugging

* add readme info about using secret for registry passwords

* fix writing the secret key to hal's config

* Enable artifacts by default

* Allow Halyard and Spinnaker SAs to be configurable

* Use custom SA in halyard SS if passed

* Default to version 1.8.4 of Spinnaker

* Simplify ingress values, fix nodeports for svcs

* Slight change; moving 'host' to be under Ingress resource

* Updater Spinnaker to 1.8.5

* Make nodeport exposure idempotent

* Add newline

* Add newline

* Add newline

* Add newline

Signed-off-by: ishabalin <ishabalin@ninthdecimal.com>
marekaf pushed a commit to marekaf/charts that referenced this pull request Sep 4, 2018
* Use Halyard for Spinnaker installation

* Bump spinnaker version in Chart.yaml

* Add whitespace in helpers

* Remove dependency on Jenkins

* Fix cleanup

* Fix NOTES command for exec into hal

* No longer need override_config_map helper

* Simplify cleanup job

* Remove halyard-pvc empty file

* Update README

* Add lwander as maintainer of Spinnaker chart

* Make halyard-local config simpler

* Remove type: Opaque from kubeconfig map

* Mount GCS key on Halyard statefulset

* Fix credentials when using kubeConfig.enabled

* Allow custom config to be provided at install time.

* Fix port-forward commands in README

* Configure Halyard's RBAC

* Use redis deployed by helm

* Use Deck to route /gate requests

* Fix RBAC in default configuration

* Scope template helper names

* Helper for resource metadata

* Enable spinnakerFeatureFlags

* Fix deck service name

* Basic S3 support.

setting access keys is optional, if they are unset, the implicit
credentials from your environment will be used.

* Smallish updates.

* Runs the install script with bash -xe for log output.
* Makes the install script somewhat idempotent in the case of chart
  upgrades on an existing halyard config state.
* Fully qualifies serviceaccount names.
* Whitespace fixes.

* Enable 'jobs' feature flag by default

* Fix typo in README

* Follow RBAC best practices for chart

* Remove trailing space

* update helm dependencies

Update minio and redis deps to be their current releases

* support username/password for docker registry

* fix variable condition for registrysecret

* remove rogue sleep command from debugging

* add readme info about using secret for registry passwords

* fix writing the secret key to hal's config

* Enable artifacts by default

* Allow Halyard and Spinnaker SAs to be configurable

* Use custom SA in halyard SS if passed

* Default to version 1.8.4 of Spinnaker

* Simplify ingress values, fix nodeports for svcs

* Slight change; moving 'host' to be under Ingress resource

* Updater Spinnaker to 1.8.5

* Make nodeport exposure idempotent

* Add newline

* Add newline

* Add newline

* Add newline

Signed-off-by: Marek Bartik <mab@revolgy.com>
Signed-off-by: Marek Bartik <bartimar6@gmail.com>
aba182 pushed a commit to aba182/charts that referenced this pull request Sep 7, 2018
* Use Halyard for Spinnaker installation

* Bump spinnaker version in Chart.yaml

* Add whitespace in helpers

* Remove dependency on Jenkins

* Fix cleanup

* Fix NOTES command for exec into hal

* No longer need override_config_map helper

* Simplify cleanup job

* Remove halyard-pvc empty file

* Update README

* Add lwander as maintainer of Spinnaker chart

* Make halyard-local config simpler

* Remove type: Opaque from kubeconfig map

* Mount GCS key on Halyard statefulset

* Fix credentials when using kubeConfig.enabled

* Allow custom config to be provided at install time.

* Fix port-forward commands in README

* Configure Halyard's RBAC

* Use redis deployed by helm

* Use Deck to route /gate requests

* Fix RBAC in default configuration

* Scope template helper names

* Helper for resource metadata

* Enable spinnakerFeatureFlags

* Fix deck service name

* Basic S3 support.

setting access keys is optional, if they are unset, the implicit
credentials from your environment will be used.

* Smallish updates.

* Runs the install script with bash -xe for log output.
* Makes the install script somewhat idempotent in the case of chart
  upgrades on an existing halyard config state.
* Fully qualifies serviceaccount names.
* Whitespace fixes.

* Enable 'jobs' feature flag by default

* Fix typo in README

* Follow RBAC best practices for chart

* Remove trailing space

* update helm dependencies

Update minio and redis deps to be their current releases

* support username/password for docker registry

* fix variable condition for registrysecret

* remove rogue sleep command from debugging

* add readme info about using secret for registry passwords

* fix writing the secret key to hal's config

* Enable artifacts by default

* Allow Halyard and Spinnaker SAs to be configurable

* Use custom SA in halyard SS if passed

* Default to version 1.8.4 of Spinnaker

* Simplify ingress values, fix nodeports for svcs

* Slight change; moving 'host' to be under Ingress resource

* Updater Spinnaker to 1.8.5

* Make nodeport exposure idempotent

* Add newline

* Add newline

* Add newline

* Add newline

Signed-off-by: aba182 <ajwilhel@gmail.com>
aba182 pushed a commit to aba182/charts that referenced this pull request Sep 7, 2018
* Use Halyard for Spinnaker installation

* Bump spinnaker version in Chart.yaml

* Add whitespace in helpers

* Remove dependency on Jenkins

* Fix cleanup

* Fix NOTES command for exec into hal

* No longer need override_config_map helper

* Simplify cleanup job

* Remove halyard-pvc empty file

* Update README

* Add lwander as maintainer of Spinnaker chart

* Make halyard-local config simpler

* Remove type: Opaque from kubeconfig map

* Mount GCS key on Halyard statefulset

* Fix credentials when using kubeConfig.enabled

* Allow custom config to be provided at install time.

* Fix port-forward commands in README

* Configure Halyard's RBAC

* Use redis deployed by helm

* Use Deck to route /gate requests

* Fix RBAC in default configuration

* Scope template helper names

* Helper for resource metadata

* Enable spinnakerFeatureFlags

* Fix deck service name

* Basic S3 support.

setting access keys is optional, if they are unset, the implicit
credentials from your environment will be used.

* Smallish updates.

* Runs the install script with bash -xe for log output.
* Makes the install script somewhat idempotent in the case of chart
  upgrades on an existing halyard config state.
* Fully qualifies serviceaccount names.
* Whitespace fixes.

* Enable 'jobs' feature flag by default

* Fix typo in README

* Follow RBAC best practices for chart

* Remove trailing space

* update helm dependencies

Update minio and redis deps to be their current releases

* support username/password for docker registry

* fix variable condition for registrysecret

* remove rogue sleep command from debugging

* add readme info about using secret for registry passwords

* fix writing the secret key to hal's config

* Enable artifacts by default

* Allow Halyard and Spinnaker SAs to be configurable

* Use custom SA in halyard SS if passed

* Default to version 1.8.4 of Spinnaker

* Simplify ingress values, fix nodeports for svcs

* Slight change; moving 'host' to be under Ingress resource

* Updater Spinnaker to 1.8.5

* Make nodeport exposure idempotent

* Add newline

* Add newline

* Add newline

* Add newline

Signed-off-by: aba182 <ajwilhel@gmail.com>
Jnig pushed a commit to Jnig/charts that referenced this pull request Nov 13, 2018
* Use Halyard for Spinnaker installation

* Bump spinnaker version in Chart.yaml

* Add whitespace in helpers

* Remove dependency on Jenkins

* Fix cleanup

* Fix NOTES command for exec into hal

* No longer need override_config_map helper

* Simplify cleanup job

* Remove halyard-pvc empty file

* Update README

* Add lwander as maintainer of Spinnaker chart

* Make halyard-local config simpler

* Remove type: Opaque from kubeconfig map

* Mount GCS key on Halyard statefulset

* Fix credentials when using kubeConfig.enabled

* Allow custom config to be provided at install time.

* Fix port-forward commands in README

* Configure Halyard's RBAC

* Use redis deployed by helm

* Use Deck to route /gate requests

* Fix RBAC in default configuration

* Scope template helper names

* Helper for resource metadata

* Enable spinnakerFeatureFlags

* Fix deck service name

* Basic S3 support.

setting access keys is optional, if they are unset, the implicit
credentials from your environment will be used.

* Smallish updates.

* Runs the install script with bash -xe for log output.
* Makes the install script somewhat idempotent in the case of chart
  upgrades on an existing halyard config state.
* Fully qualifies serviceaccount names.
* Whitespace fixes.

* Enable 'jobs' feature flag by default

* Fix typo in README

* Follow RBAC best practices for chart

* Remove trailing space

* update helm dependencies

Update minio and redis deps to be their current releases

* support username/password for docker registry

* fix variable condition for registrysecret

* remove rogue sleep command from debugging

* add readme info about using secret for registry passwords

* fix writing the secret key to hal's config

* Enable artifacts by default

* Allow Halyard and Spinnaker SAs to be configurable

* Use custom SA in halyard SS if passed

* Default to version 1.8.4 of Spinnaker

* Simplify ingress values, fix nodeports for svcs

* Slight change; moving 'host' to be under Ingress resource

* Updater Spinnaker to 1.8.5

* Make nodeport exposure idempotent

* Add newline

* Add newline

* Add newline

* Add newline

Signed-off-by: Jakob Niggel <info@jakobniggel.de>
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.