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

fix(cf): fix error description with v2 routes/route-mappings #5119

Merged
merged 10 commits into from
Nov 25, 2020
Merged

fix(cf): fix error description with v2 routes/route-mappings #5119

merged 10 commits into from
Nov 25, 2020

Conversation

zachsmith1
Copy link
Contributor

This problem caused server groups to not deploy and error out if you had a route in your manifest. It also rendered the map/unmap route stages useless. This change needs to be backported to 1.23, 1.22. I spent a decent amount of time trying to figure out why this worked in previous versions and was unable to find the causation. I've added tests to cover the new logic and ensure proper mapping. This class will eventually go away as we move towards okhttp3 and dynamic api binding for services based on the supported cf version.

@kevinawoo kevinawoo merged commit f9be9d7 into spinnaker:master Nov 25, 2020
@kevinawoo kevinawoo added the backport-candidate Add to PRs to designate release branch patch candidates. label Nov 25, 2020
@dogonthehorizon dogonthehorizon removed the backport-candidate Add to PRs to designate release branch patch candidates. label Dec 1, 2020
@jburnard9464
Copy link

Just to clarify from a backport perspective. This was working in version 1.21. It started failing in version 1.22.

@dogonthehorizon
Copy link
Member

@Mergifyio backport release-1.22.x release-1.23.x

@mergify mergify bot mentioned this pull request Dec 1, 2020
Merged
mergify bot pushed a commit that referenced this pull request Dec 1, 2020
@zachsmith1
fix(cf): fix error description with v2 routes/route-mappings (#5119)
c485ec0 
(cherry picked from commit f9be9d7)
mergify bot pushed a commit that referenced this pull request Dec 1, 2020
@zachsmith1
fix(cf): fix error description with v2 routes/route-mappings (#5119)
ec5544e 
(cherry picked from commit f9be9d7)
@mergify mergify bot mentioned this pull request Dec 1, 2020
Merged
@mergify
Copy link
Contributor

mergify bot commented Dec 1, 2020

Command backport release-1.22.x release-1.23.x: success

Backports have been created

mergify bot added a commit that referenced this pull request Dec 1, 2020
@mergify @zachsmith1
fix(cf): fix error description with v2 routes/route-mappings (#5119) (#…
f378293 
…5126)

(cherry picked from commit f9be9d7)

Co-authored-by: Zach Smith <33258732+zachsmith1@users.noreply.github.com>
mergify bot added a commit that referenced this pull request Dec 1, 2020
@mergify @zachsmith1
fix(cf): fix error description with v2 routes/route-mappings (#5119) (#…
0d6869b 
…5127)

(cherry picked from commit f9be9d7)

Co-authored-by: Zach Smith <33258732+zachsmith1@users.noreply.github.com>
@zachsmith1
Copy link
Contributor Author

Fixes this -> spinnaker/spinnaker#6163

@zachsmith1 zachsmith1 mentioned this pull request Dec 1, 2020
Closed
@zachsmith1 zachsmith1 deleted the fix/cferrordescription branch December 1, 2020 18:27
@spinnakerbot
Copy link
Contributor

The following commits need their title changed:

Please format your commit title into the form:

<type>(<scope>): <subject>, e.g. fix(kubernetes): address NPE in status check

This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevinawoo kevinawoo kevinawoo approved these changes

Assignees
No one assigned
Labels
target-release/1.24
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zachsmith1 @jburnard9464 @dogonthehorizon @spinnakerbot @kevinawoo