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: allow setting default subnet for custom vpc #4171

Merged
merged 14 commits into from
Aug 23, 2024

Conversation

cnvergence
Copy link
Contributor

@cnvergence cnvergence commented Jun 13, 2024

Pull Request

What type of this PR

Examples of user facing changes:

  • Features
  • Bug fixes
  • Docs
  • Tests

Which issue(s) this PR fixes

Fixes #4172

@zhangzujian zhangzujian added the feature New network feature label Jun 14, 2024
@cnvergence cnvergence marked this pull request as ready for review July 10, 2024 12:58
@cnvergence
Copy link
Contributor Author

/retest

@zbb88888
Copy link
Collaborator

the crd yamls should be updated too

@cnvergence
Copy link
Contributor Author

right, do you have maybe a make target or script to generate this?

@zbb88888
Copy link
Collaborator

right, do you have maybe a make target or script to generate this?

maybe later,

@cnvergence cnvergence force-pushed the default-subnet-custom-vpc branch from 7f7b676 to 7166eeb Compare July 12, 2024 17:20
@cnvergence
Copy link
Contributor Author

ptal @bobz965

charts/kube-ovn/templates/kube-ovn-crd.yaml Show resolved Hide resolved
dist/images/install.sh Show resolved Hide resolved
pkg/controller/namespace.go Outdated Show resolved Hide resolved
pkg/util/const.go Outdated Show resolved Hide resolved
@cnvergence cnvergence force-pushed the default-subnet-custom-vpc branch from c582bdf to d512d5f Compare August 1, 2024 11:03
@cnvergence cnvergence requested a review from zbb88888 August 1, 2024 11:04
@cnvergence
Copy link
Contributor Author

PR updated @bobz965 PTAL at your convienience

pkg/util/const.go Outdated Show resolved Hide resolved
@zbb88888
Copy link
Collaborator

@oilbeater could you please help review?

@zbb88888 zbb88888 requested a review from oilbeater August 12, 2024 02:17
@oilbeater
Copy link
Collaborator

I think we do not need another annotation for namespace, add the ovn.kubernetes.io/logical_switch should be enough for further pod processing just as in the default VPC. What do you think? @cnvergence @bobz965

@zbb88888
Copy link
Collaborator

ovn.kubernetes.io/logical_switch

oh, I saw the code:
namespace already has the ovn.kubernetes.io/logical_switch

image

@cnvergence
Copy link
Contributor Author

@oilbeater @bobz965 thanks for the review
Thinking out loud, it should work with ovn.kubernetes.io/logical_switch by just overwriting the logical switch slice during namespace processing, let's try this.

@cnvergence cnvergence force-pushed the default-subnet-custom-vpc branch from f28cbaf to bcf37b6 Compare August 12, 2024 17:35
@cnvergence cnvergence requested a review from zbb88888 August 12, 2024 17:35
pkg/controller/namespace.go Outdated Show resolved Hide resolved
pkg/controller/vpc.go Show resolved Hide resolved
Signed-off-by: Karol Szwaj <[email protected]>
@zbb88888
Copy link
Collaborator

please add some doc about the vpc spec, and clarify the usage details between vpc spec default subnet and namespace annotation logical switch list.

@cnvergence
Copy link
Contributor Author

Thanks @bobz965, I will add the docs PR asap

@cnvergence
Copy link
Contributor Author

PTAL @oilbeater

@oilbeater oilbeater merged commit 6f669c7 into kubeovn:master Aug 23, 2024
60 of 62 checks passed
@oilbeater
Copy link
Collaborator

@cnvergence Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New network feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Allow setting default subnet for custom vpc
4 participants