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

2169 rolling deploy timeout issue #2170

Merged
merged 2 commits into from
Jul 2, 2021

Conversation

sethboyles
Copy link
Member

Does this PR modify CLI v6 or v7?

v8

Description of the Change

Resolves #2169

Currently, when a push with strategy=rolling takes longer than CF_STARTUP_TIMEOUT, the CLI exits 1 but the deployment continues to attempt bringing up new instances of the app indefinitely. With this change, when the CLI times out, it makes an additional call to CAPI to cancel the deployment, mirroring behavior similar to a non-rolling push.

How Urgent Is The Change?

not urgent, but an annoying bug

issue #2169

Co-authored-by: Teal Stannard <tstannard@pivotal.io>
Co-authored-by: Seth Boyles <sboyles@pivotal.io>
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/178309610

The labels on this github issue will be updated when the story is started.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented May 26, 2021

CLA Signed

The committers are authorized under a signed CLA.

@sethboyles
Copy link
Member Author

sethboyles commented May 26, 2021

@tstannard please authorize the CLA 🌝

canceled

Co-authored-by: Jenna Goldstrich <jgoldstrich@pivotal.io>
Co-authored-by: Seth Boyles <sboyles@pivotal.io>
@tstannard tstannard force-pushed the 2169-rolling-deploy-timeout-issue branch from cd86a64 to 8744231 Compare May 27, 2021 18:05
@tstannard
Copy link

hi CLI,
This is ready to merge :)
Please let us know if you would like a meeting to sync on it
Teal & @JenGoldstrich

@a-b
Copy link
Member

a-b commented May 27, 2021

@tstannard and @JenGoldstrich these changes are only applicable to the master (V8), right?

@JenGoldstrich
Copy link
Contributor

@a-b we actually want these changes on v7 and v8, as they mostly just make v7 rolling push failures a little more graceful. Should we open a second PR for to the v7 branch? How would y'all prefer we submit PRs that are intended for v7 and v8?

@a-b
Copy link
Member

a-b commented May 27, 2021

Thanks for the clarification. No need to make additional PR we will take care of that.

Copy link
Member

@a-b a-b left a comment

Choose a reason for hiding this comment

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

LGTM

@JenGoldstrich JenGoldstrich merged commit 5cfbf04 into master Jul 2, 2021
@JenGoldstrich
Copy link
Contributor

Merging this based on Alex signing off on it.

JenGoldstrich pushed a commit that referenced this pull request Jul 2, 2021
Merge pull request #2170 from cloudfoundry/2169-rolling-deploy-timeout-issue

2169 rolling deploy timeout issue
@a-b a-b deleted the 2169-rolling-deploy-timeout-issue branch June 12, 2023 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cancel rolling deployments when the CF_STARTUP_TIMEOUT has been reached
5 participants