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

Use Networking/v1 as default group version in ingress manager #2175

Merged
merged 2 commits into from
Jul 13, 2023
Merged

Use Networking/v1 as default group version in ingress manager #2175

merged 2 commits into from
Jul 13, 2023

Conversation

PrasadG193
Copy link
Contributor

@PrasadG193 PrasadG193 commented Jul 13, 2023 •
edited

Change Overview

This PR removes the Discovery APIs from ingress manager.
Now it uses networking v1 as default group version to manager ingress resources.

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Issues

  • fixes #issue-number

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@PrasadG193
Make Networking/v1 as default group version for ingress
488fbac 
Signed-off-by: Prasad Ghangal <prasad.ghangal@gmail.com>
@github-actions
Copy link
Contributor

Thanks for submitting this pull request 🎉. The team will review it soon and get back to you.

If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document.

@infraq infraq added this to In Progress in Kanister Jul 13, 2023
@PrasadG193 PrasadG193 marked this pull request as ready for review July 13, 2023 18:29
Kanister automation moved this from In Progress to Reviewer approved Jul 13, 2023
onkarbhat
onkarbhat approved these changes Jul 13, 2023
View reviewed changes
Copy link
Contributor

@onkarbhat onkarbhat left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

pavannd1
pavannd1 reviewed Jul 13, 2023
View reviewed changes
Copy link
Contributor

@pavannd1 pavannd1 left a comment

Choose a reason for hiding this comment

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

Note for the future: Not sure if we still need the IngressManager. We could probably use NetworkingV1 directly everywhere.

@viveksinghggits
Copy link
Contributor

viveksinghggits commented Jul 13, 2023 •
edited

Nit: Should we remove IsResAvailableInGroupVersion from discover.go? It's not being used anywhere now.

@pavannd1
Copy link
Contributor

Merging this now. Let's follow-up on the two comments soon.

@pavannd1 pavannd1 added the kueue label Jul 13, 2023
@mergify mergify bot merged commit a66aec9 into master Jul 13, 2023
14 checks passed
Kanister automation moved this from Reviewer approved to Done Jul 13, 2023
@mergify mergify bot deleted the ing-manager-refactor branch July 13, 2023 23:35
mellon-collie pushed a commit that referenced this pull request Jul 20, 2023
@PrasadG193 @mergify @mellon-collie
Make Networking/v1 as default group version for ingress (#2175)
f1b298c 
Signed-off-by: Prasad Ghangal <prasad.ghangal@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pavannd1 pavannd1 pavannd1 left review comments

@onkarbhat onkarbhat onkarbhat approved these changes

@viveksinghggits viveksinghggits viveksinghggits approved these changes

Assignees

@onkarbhat onkarbhat

@viveksinghggits viveksinghggits

Labels
kueue
Projects
Kanister
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@PrasadG193 @viveksinghggits @pavannd1 @onkarbhat