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

examples: Fix YAML error backendRefs in HTTP Header Modifier #27871

Merged
merged 1 commit into from Sep 20, 2023
Merged

examples: Fix YAML error backendRefs in HTTP Header Modifier #27871

merged 1 commit into from Sep 20, 2023

Conversation

haiyuewa
Copy link
Contributor

@haiyuewa haiyuewa commented Sep 1, 2023

The commit bb50725 ("Header Modifier and Splitting use cases") used wrong 'backendRefs' service name setting for Header Modifier HTTPRoute.

kubectl describe HTTPRoute header-http-echo
Message: Service "echo" not found
Observed Generation: 1
Reason: BackendNotFound
Status: False

The demo has two services: "echo-1" with 8080, "echo-2" with 8090. Fix the wrong name:

kubectl describe HTTPRoute header-http-echo
Message: Service reference is valid
Observed Generation: 1
Reason: ResolvedRefs
Status: True
Type: ResolvedRefs

examples: Fix YAML error backendRefs in HTTP Header Modifier

@haiyuewa haiyuewa requested a review from a team as a code owner September 1, 2023 08:13
@maintainer-s-little-helper
Copy link

Commit 34cd9a88c82755bf24c6135f1b953a96b90ff6ad does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Sep 1, 2023
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Sep 1, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Sep 1, 2023
@haiyuewa
Copy link
Contributor Author

Hi @nvibert , I did the setup step by step as you wrote, just found one mirror typo issue. Have time to review my change ? ;-)

@haiyuewa haiyuewa changed the title docs: Fix Gateway API Header Modifier use case YAML error examples: Fix YAML error backendRefs in HTTP Header Modifier Sep 13, 2023
@nvibert
Copy link
Contributor

nvibert commented Sep 13, 2023

Hi @nvibert , I did the setup step by step as you wrote, just found one mirror typo issue. Have time to review my change ? ;-)

Let me check 👍

@nvibert
Copy link
Contributor

nvibert commented Sep 13, 2023

Yep, @haiyuewa - you're absolutely correct. Well spotted !

I don't have reviewers rights but if I had, I would approve it 😅

The use case "HTTP Header Modifier" has wrong 'backendRefs' service name:
  $kubectl describe HTTPRoute header-http-echo
      Message:               Service "echo" not found
      Observed Generation:   1
      Reason:                BackendNotFound
      Status:                False

The right service name should be "echo-1", which has port number 8080:
  $kubectl describe HTTPRoute header-http-echo
      Message:               Service reference is valid
      Observed Generation:   1
      Reason:                ResolvedRefs
      Status:                True
      Type:                  ResolvedRefs

Fixes: bb50725 ("Header Modifier and Splitting use cases")
Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
@haiyuewa
Copy link
Contributor Author

/test

@ldelossa ldelossa added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Sep 20, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Sep 20, 2023
@ldelossa
Copy link
Contributor

thanks for contribution!

@ldelossa ldelossa merged commit 8db9095 into cilium:main Sep 20, 2023
61 checks passed
@haiyuewa haiyuewa deleted the fix-gateway-api-use-case-yaml-error branch September 20, 2023 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants