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.

Sign up for GitHub

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

Jump to bottom

feat(kubernetes): permit multiple ReplicaSets to be deployed with a s… #3264

Merged
merged 2 commits into from
Oct 29, 2019
Merged

feat(kubernetes): permit multiple ReplicaSets to be deployed with a s… #3264

merged 2 commits into from
Oct 29, 2019

Conversation

maggieneterval
Copy link
Contributor

…ingle rollout strategy

Closes spinnaker/spinnaker#5025

Corresponding PRs:
Deck: spinnaker/deck#7574
Clouddriver: spinnaker/clouddriver#4126

  • Previously, we restricted users to deploying a single ReplicaSet when configuring Spinnaker-managed rollout strategies. This removes that restriction so that users generating a multi-manifest with tools like Kustomize can configure multiple ReplicaSets to be deployed with the same rollout strategy.
  • The cost of removing this restriction is potential confusion in the case where a user did not intend to apply the rollout strategy to multiple workloads, but I think this is acceptable and we can consider further warning messages if we receive feedback that this is confusing.
  • This change does not allow you to configure multiple rollout strategies and associate each with a workload. Each Deploy (Manifest) stage has one associated rollout strategy config that will be associated with each ReplicaSet it deploys.

dca05054-1db8-493c-8fa6-67c7749a0899

This was referenced Oct 28, 2019
Merged
Merged
ezimanyi
ezimanyi approved these changes Oct 29, 2019
View reviewed changes
Copy link
Contributor

@ezimanyi ezimanyi left a comment

Choose a reason for hiding this comment

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

👍 👥 👐 🏐 🎈 🎉

...ain/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStage.java
@@ -86,46 +86,44 @@ private void deleteOldManifests(Map parentContext, StageGraphBuilder graph) {

private void addStagesForOldManifests(
Map parentContext, StageGraphBuilder graph, String stageType) {
Map deployedManifest = getNewManifest(parentContext);
List<Map<String, ?>> deployedManifests = getNewManifests(parentContext);
Copy link
Contributor

Choose a reason for hiding this comment

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

💪 ⌨️
💯 🏅

@maggieneterval maggieneterval added the ready to merge Approved and ready for merge label Oct 29, 2019
@mergify mergify bot merged commit e2d395a into spinnaker:master Oct 29, 2019
@mergify mergify bot added the auto merged Merged automatically by a bot label Oct 29, 2019
sergiopena pushed a commit to sergiopena/orca that referenced this pull request Dec 13, 2019
@maggieneterval
feat(kubernetes): permit multiple ReplicaSets to be deployed with a s…
1fc6677 
…ingle rollout strategy (spinnaker#3264)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ezimanyi ezimanyi ezimanyi approved these changes

Assignees
No one assigned
Labels
auto merged Merged automatically by a bot ready to merge Approved and ready for merge target-release/1.17
Projects
None yet
Milestone
No milestone
3 participants
@maggieneterval @ezimanyi @spinnakerbot