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

Port: "retryOn" configuration on ServiceRouter CRD #3308

Merged
merged 2 commits into from
Dec 7, 2023
Merged

Port: "retryOn" configuration on ServiceRouter CRD #3308

merged 2 commits into from
Dec 7, 2023

Conversation

DanStough
Copy link
Contributor

Changes proposed in this PR:

  • This is a port of Adding support for retryOn in Service Router spec #2435 into a PR that can run on the HashiCorp CI. It adds support for the retryOn field that is missing from the CRD.
  • I ensured everything was re-generated cleanly. This added the missing license header to some missing files.

Closes #3207

How I've tested this PR: Unit tests.

How I expect reviewers to test this PR: 🔍

Checklist:

@DanStough DanStough added backport/1.1.x Backport to release/1.1.x branch backport/1.2.x This release branch is no longer active. backport/1.3.x labels Dec 6, 2023
@DanStough DanStough changed the title Dans/service resolver retryon Port: "retryOn" configuration on ServiceRouter CRD Dec 6, 2023
@DanStough DanStough force-pushed the dans/service-resolver-retryon branch from 86160cf to 16a4826 Compare December 6, 2023 15:55
zalimeni
zalimeni approved these changes Dec 6, 2023
View reviewed changes
Copy link
Member

@zalimeni zalimeni left a comment

Choose a reason for hiding this comment

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

LGTM.

Non-blocking question: would we want to explicitly link to the docs for the list of supported conditions, or do we typically assume folks know to go looking for the corresponding Consul config docs?

.changelog/3308.txt Outdated Show resolved Hide resolved
@DanStough DanStough mentioned this pull request Dec 6, 2023
Closed
2 tasks
@DanStough
Copy link
Contributor Author

@zalimeni I linked to the conditions earlier in the changelog, if that's where you're referring to?

@DanStough DanStough force-pushed the dans/service-resolver-retryon branch from e421cc9 to 3200313 Compare December 6, 2023 16:02
@zalimeni
Copy link
Member

zalimeni commented Dec 6, 2023 •
edited
Loading

@zalimeni I linked to the conditions earlier in the changelog, if that's where you're referring to?

@DanStough sorry, I was more thinking our own inline docs here, for the sake of folks unfamiliar w/ the field's semantics - not sure whether "check the Consul docs" is typically implied

@DanStough
Copy link
Contributor Author

I would say it's typically implied but no harm in adding it ✍️

@DanStough DanStough force-pushed the dans/service-resolver-retryon branch from 3200313 to f828314 Compare December 6, 2023 16:09
@DanStough DanStough force-pushed the dans/service-resolver-retryon branch from f828314 to 106905a Compare December 6, 2023 22:22
thisisnotashwin
thisisnotashwin approved these changes Dec 7, 2023
View reviewed changes
Copy link
Contributor

@thisisnotashwin thisisnotashwin left a comment

Choose a reason for hiding this comment

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

This looks good. Have a suggestion but you can ignore it if it doesnt seem valueable.

control-plane/api/v1alpha1/servicerouter_types.go Show resolved Hide resolved
@DanStough DanStough force-pushed the dans/service-resolver-retryon branch from 106905a to 78bc22a Compare December 7, 2023 16:36
@DanStough DanStough merged commit 2cee7db into main Dec 7, 2023
44 of 48 checks passed
@DanStough DanStough deleted the dans/service-resolver-retryon branch December 7, 2023 20:23
This was referenced Dec 7, 2023
Merged
Merged
Merged
DanStough added a commit that referenced this pull request Dec 7, 2023
@DanStough @ilpianista
Port: "retryOn" configuration on ServiceRouter CRD (#3308)
712a588 
* feat(crds): add support for retryOn in service router

Co-authored-by: ilpianista <[email protected]>

---------

Co-authored-by: ilpianista <[email protected]>
DanStough added a commit that referenced this pull request Dec 7, 2023
@DanStough @ilpianista
Port: "retryOn" configuration on ServiceRouter CRD (#3308)
cba96ce 
* feat(crds): add support for retryOn in service router

Co-authored-by: ilpianista <[email protected]>

---------

Co-authored-by: ilpianista <[email protected]>
DanStough added a commit that referenced this pull request Dec 7, 2023
@DanStough @ilpianista
Port: "retryOn" configuration on ServiceRouter CRD (#3308)
703e5b3 
* feat(crds): add support for retryOn in service router

Co-authored-by: ilpianista <[email protected]>

---------

Co-authored-by: ilpianista <[email protected]>
DanStough added a commit that referenced this pull request Dec 7, 2023
@hc-github-team-consul-core @DanStough @ilpianista
Backport of Port: "retryOn" configuration on ServiceRouter CRD into r…
0b10017 
…elease/1.2.x (#3324)

Port: "retryOn" configuration on ServiceRouter CRD (#3308)

* feat(crds): add support for retryOn in service router



---------

Co-authored-by: Dan Stough <[email protected]>
Co-authored-by: ilpianista <[email protected]>
DanStough added a commit that referenced this pull request Dec 8, 2023
@hc-github-team-consul-core @DanStough @ilpianista
Backport of Port: "retryOn" configuration on ServiceRouter CRD into r…
2045f92 
…elease/1.3.x (#3325)

Port: "retryOn" configuration on ServiceRouter CRD (#3308)

* feat(crds): add support for retryOn in service router



---------

Co-authored-by: Dan Stough <[email protected]>
Co-authored-by: ilpianista <[email protected]>
DanStough added a commit that referenced this pull request Dec 8, 2023
@hc-github-team-consul-core @DanStough @ilpianista
Backport of Port: "retryOn" configuration on ServiceRouter CRD into r…
e0b4d48 
…elease/1.1.x (#3323)

Port: "retryOn" configuration on ServiceRouter CRD (#3308)

* feat(crds): add support for retryOn in service router



---------

Co-authored-by: Dan Stough <[email protected]>
Co-authored-by: ilpianista <[email protected]>
jmurret pushed a commit that referenced this pull request Dec 8, 2023
@DanStough @ilpianista @jmurret
Port: "retryOn" configuration on ServiceRouter CRD (#3308)
634be2c 
* chore: add license to gateways files

* feat(crds): add support for retryOn in service router

Co-authored-by: ilpianista <[email protected]>

---------

Co-authored-by: ilpianista <[email protected]>
sarahalsmiller pushed a commit that referenced this pull request Jan 5, 2024
@DanStough @ilpianista @sarahalsmiller
Port: "retryOn" configuration on ServiceRouter CRD (#3308)
925a1c6 
* chore: add license to gateways files

* feat(crds): add support for retryOn in service router

Co-authored-by: ilpianista <[email protected]>

---------

Co-authored-by: ilpianista <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thisisnotashwin thisisnotashwin thisisnotashwin approved these changes

@zalimeni zalimeni zalimeni approved these changes

Assignees
No one assigned
Labels
backport/1.1.x Backport to release/1.1.x branch backport/1.2.x This release branch is no longer active. backport/1.3.x
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"retryOn" configuration on ServiceRouter CRD
3 participants
@DanStough @zalimeni @thisisnotashwin