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

feat: add support for host pass in upstream crd #1889

Merged

Conversation

ikatlinsky
Copy link
Contributor

@ikatlinsky ikatlinsky commented Jul 6, 2023

Type of change:

  • New feature provided
  • Documentation
  • CI/CD or Tests

What this PR does / why we need it:

As per feature request #1661

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible?

@ikatlinsky ikatlinsky marked this pull request as draft July 6, 2023 14:42
@ikatlinsky ikatlinsky marked this pull request as ready for review July 6, 2023 17:17
@ikatlinsky ikatlinsky changed the title feat: initial support for host pass in upstream, no e2e feat: add support for host pass in upstream crd Jul 6, 2023
@ikatlinsky
Copy link
Contributor Author

ikatlinsky commented Jul 7, 2023

Will need some time to investigate why e2e tests are failing, I checked them in the master branch and they are also failing quite randomly (example for external-service.go):

Summarizing 3 Failures:
  [FAIL] suite-features: external services update function:  [It] should be able to create the ApisixUpstream later
  /Users/i.katlinsky/go/pkg/mod/github.com/gavv/httpexpect/v2@v2.15.0/reporter.go:24
  [FAIL] suite-features: external services complex usage:  [It] should be able to use backends and upstreams together
  /Users/i.katlinsky/Documents/sandbox/apisix-ingress-controller/test/e2e/suite-features/external-service.go:407
  [FAIL] suite-features: external services complex usage:  [It] should be able to access multiple external services
  /Users/i.katlinsky/go/pkg/mod/github.com/gavv/httpexpect/v2@v2.15.0/reporter.go:24

@tao12345666333 is it a known issue?

@tao12345666333
Copy link
Member

no

@ikatlinsky
Copy link
Contributor Author

here is an example of the failing build on master branch today

https://github.com/apache/apisix-ingress-controller/actions/runs/5491609801

@tao12345666333
Copy link
Member

let me take a look

@tao12345666333
Copy link
Member

I will take some time to solve the CI problems. After all test passed, we can merge this one.

@ikatlinsky
Copy link
Contributor Author

nice, make me know when CI problems will be resolved, so i can run test on my side too

@adam-huganir
Copy link

let me take a look

I hate to bug you on this @tao12345666333 , but i was wondering if you had any idea when you might have time to take a look at the tests. This is the last feature we need to have purely crd based deployment and we are very excited it is already in the pipeline to deployment. Thanks for all the work so far on this

@tao12345666333
Copy link
Member

Please merge the master branch first. All test cases be fixed. Thanks

@ikatlinsky
Copy link
Contributor Author

@tao12345666333 merged master into my branch, but at least locally, tests are randomly failing for me with a 504 gateway timeout error. That may not be the case in GitHub actions.

@ikatlinsky
Copy link
Contributor Author

@tao12345666333 as i see builds on master are still failing https://github.com/apache/apisix-ingress-controller/actions/runs/5767636410

@tao12345666333
Copy link
Member

yes. I will fix the test cases in #1918

@tao12345666333
Copy link
Member

@ikatlinsky Please merge the latest code from the master branch. We have fixed the CI issues.

@ikatlinsky
Copy link
Contributor Author

@tao12345666333 done

Copy link
Member

@tao12345666333 tao12345666333 left a comment

Choose a reason for hiding this comment

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

LGTM

@tao12345666333 tao12345666333 added this to the v1.7.0 milestone Aug 26, 2023
@tao12345666333 tao12345666333 merged commit 7b3deb5 into apache:master Aug 26, 2023
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants