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

Fix/blue green deploy #4414

Merged
merged 5 commits into from
Mar 23, 2023
Merged

Conversation

kkotula
Copy link
Contributor

@kkotula kkotula commented Mar 21, 2023

This PR addresses an interesting bug observed while using Blue/Green (Red/Black) deployment.

If the first replica set deployment fails (e.g. due to an incorrect image), a consecutive deployment with a fixed image name would also fail. This was due to one of the new deployment stages disabling the previous deployment. The DisableManifestStage waits for the manifest to stabilize, which never happens since the original pods were not running in the first place.

In this PR, I attempt to tackle the problem by verifying the old manifest status. When it is not stable but not failed, we are dealing with the situation described above.

I also made slight refactoring to the existing code. The primary focus of the refactoring was to extract new helper classes that encapsulate some parts of the existing logic.

@kkotula kkotula marked this pull request as draft March 21, 2023 12:55
Copy link
Contributor

@ovidiupopa07 ovidiupopa07 left a comment

Choose a reason for hiding this comment

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

Lgtm

@kkotula kkotula marked this pull request as ready for review March 23, 2023 08:24
@ovidiupopa07 ovidiupopa07 added the ready to merge Approved and ready for merge label Mar 23, 2023
@mergify mergify bot added the auto merged Merged automatically by a bot label Mar 23, 2023
@mergify mergify bot merged commit 5156eec into spinnaker:master Mar 23, 2023
@ovidiupopa07
Copy link
Contributor

@mergify backport release-1.27.x release-1.28.x release-1.29.x

@mergify
Copy link
Contributor

mergify bot commented Mar 27, 2023

backport release-1.27.x release-1.28.x release-1.29.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Mar 27, 2023
* fix(blue-green-deploy): removing old failed manifests

* fix(blue-green-deploy): refactoring

* fix(blue-green-deploy): refactoring

* fix(blue-green-deploy): added docs

* fix(blue-green-deploy): added test to validate new behaviour

(cherry picked from commit 5156eec)

# Conflicts:
#	orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStage.java
#	orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStageTest.java
mergify bot pushed a commit that referenced this pull request Mar 27, 2023
* fix(blue-green-deploy): removing old failed manifests

* fix(blue-green-deploy): refactoring

* fix(blue-green-deploy): refactoring

* fix(blue-green-deploy): added docs

* fix(blue-green-deploy): added test to validate new behaviour

(cherry picked from commit 5156eec)

# Conflicts:
#	orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStage.java
#	orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStageTest.java
mergify bot pushed a commit that referenced this pull request Mar 27, 2023
* fix(blue-green-deploy): removing old failed manifests

* fix(blue-green-deploy): refactoring

* fix(blue-green-deploy): refactoring

* fix(blue-green-deploy): added docs

* fix(blue-green-deploy): added test to validate new behaviour

(cherry picked from commit 5156eec)

# Conflicts:
#	orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStage.java
#	orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStageTest.java
dogonthehorizon pushed a commit that referenced this pull request Mar 28, 2023
* Fix/blue green deploy (#4414)

* fix(blue-green-deploy): removing old failed manifests

* fix(blue-green-deploy): refactoring

* fix(blue-green-deploy): refactoring

* fix(blue-green-deploy): added docs

* fix(blue-green-deploy): added test to validate new behaviour

(cherry picked from commit 5156eec)

# Conflicts:
#	orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStage.java
#	orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStageTest.java

* chore(conflict): resolved conflicts (#4422)

---------

Co-authored-by: Krzysztof Kotula <[email protected]>
Co-authored-by: ovidiupopa07 <[email protected]>
dogonthehorizon pushed a commit that referenced this pull request Mar 28, 2023
* Fix/blue green deploy (#4414)

* fix(blue-green-deploy): removing old failed manifests

* fix(blue-green-deploy): refactoring

* fix(blue-green-deploy): refactoring

* fix(blue-green-deploy): added docs

* fix(blue-green-deploy): added test to validate new behaviour

(cherry picked from commit 5156eec)

# Conflicts:
#	orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStage.java
#	orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStageTest.java

* chore(conflict): resolved conflicts (#4423)

---------

Co-authored-by: Krzysztof Kotula <[email protected]>
Co-authored-by: ovidiupopa07 <[email protected]>
dogonthehorizon pushed a commit that referenced this pull request Mar 28, 2023
* Fix/blue green deploy (#4414)

* fix(blue-green-deploy): removing old failed manifests

* fix(blue-green-deploy): refactoring

* fix(blue-green-deploy): refactoring

* fix(blue-green-deploy): added docs

* fix(blue-green-deploy): added test to validate new behaviour

(cherry picked from commit 5156eec)

# Conflicts:
#	orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStage.java
#	orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/pipeline/manifest/DeployManifestStageTest.java

* chore(conflict): resolved conflicts (#4421)

---------

Co-authored-by: Krzysztof Kotula <[email protected]>
Co-authored-by: ovidiupopa07 <[email protected]>
yugaa22 pushed a commit to OpsMx/orca-oes that referenced this pull request Jun 26, 2023
* fix(blue-green-deploy): removing old failed manifests

* fix(blue-green-deploy): refactoring

* fix(blue-green-deploy): refactoring

* fix(blue-green-deploy): added docs

* fix(blue-green-deploy): added test to validate new behaviour
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merged Merged automatically by a bot ready to merge Approved and ready for merge target-release/1.30
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants