Skip to content

feat: support LabelSelector type for NamespaceSelectors#2494

Merged
arkodg merged 4 commits intoenvoyproxy:mainfrom
deszhou:namespaceSelectors
Jan 26, 2024
Merged

feat: support LabelSelector type for NamespaceSelectors#2494
arkodg merged 4 commits intoenvoyproxy:mainfrom
deszhou:namespaceSelectors

Conversation

@deszhou
Copy link
Copy Markdown
Contributor

@deszhou deszhou commented Jan 24, 2024

What type of PR is this?

What this PR does / why we need it:

In the current configuration, the namespaceSelectors field under EnvoyGateway.Provider.Kubernetes.Wach only supports label selection using the key, similar to the Exists operator in LabelSelectorOperator.

The namespaceSelectors field can support a more comprehensive label-based selection, allowing you to specify both matchLabels and matchExpressions

namespaceSelectors:
  matchLabels:
    component: redis
  matchExpressions:
    - { key: tier, operator: In, values: [cache] }
    - { key: environment, operator: NotIn, values: [dev] }

https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#LabelSelector

Which issue(s) this PR fixes:

Fixes #2481

@deszhou
feat: support LabelSelector type for NamespaceSelectors
87c613a 
Signed-off-by: yeedove <yeedove@gmail.com>
@deszhou deszhou requested a review from a team as a code owner January 24, 2024 07:53
@deszhou
Merge branch 'main' into namespaceSelectors
35675e5 
Signed-off-by: yeedove <yeedove@gmail.com>
@deszhou deszhou closed this Jan 24, 2024
arkodg
arkodg reviewed Jan 24, 2024
View reviewed changes
api/v1alpha1/envoygateway_types.go Outdated
Copy link
Copy Markdown
Contributor

@arkodg arkodg Jan 24, 2024

Choose a reason for hiding this comment

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

should this field be renamed to namespaceSelector ?

Copy link
Copy Markdown
Contributor Author

@deszhou deszhou Jan 25, 2024

Choose a reason for hiding this comment

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

yes, done

@deszhou deszhou reopened this Jan 25, 2024
@deszhou deszhou force-pushed the namespaceSelectors branch 4 times, most recently from 2fb447c to 2a574f0 Compare January 25, 2024 09:50
@deszhou
feat: support LabelSelector type for NamespaceSelectors
bff6416 
Signed-off-by: yeedove <yeedove@gmail.com>
@deszhou deszhou force-pushed the namespaceSelectors branch from 2a574f0 to bff6416 Compare January 25, 2024 11:10
@zirain
Copy link
Copy Markdown
Member

zirain commented Jan 25, 2024

EG use squash merge model, please kindly don't force push everytime for better review.

@deszhou
Copy link
Copy Markdown
Contributor Author

deszhou commented Jan 25, 2024

EG use squash merge model, please kindly don't force push everytime for better review.

I will pay attention to this. Can you help me look at the coverage-test? thank

@zirain
Copy link
Copy Markdown
Member

zirain commented Jan 25, 2024 

--- FAIL: TestNamespaceSelectorProvider (67.82s)
    kubernetes_test.go:1372: 
        	Error Trace:	/home/runner/work/gateway/gateway/internal/provider/kubernetes/kubernetes_test.go:1372
        	Error:      	Condition never satisfied
        	Test:       	TestNamespaceSelectorProvider
2024-01-25T11:14:20.404Z	INFO	kubernetes/predicates.go:39	gatewayclass has matching controller name, processing	{"name": "test-gc"}
2024-01-25T11:14:20.404Z	INFO	kubernetes/predicates.go:43	bypassing reconciliation due to controller name	{"controller": "not.configured/controller"}
2024-01-25T11:14:20.406Z	DPANIC	kubernetes/predicates.go:123	non-string key argument passed to logging, ignoring all later arguments	{"invalid key": "not.configured/controller"}
2024-01-25T11:14:20.406Z	INFO	kubernetes/predicates.go:123	gatewayclass controller name
2024-01-25T11:14:20.406Z	INFO	kubernetes/predicates.go:124	gatewayclass name for gateway doesn't match configured name	{"namespace": "", "name": "scheduled-status-test"}
2024-01-25T11:14:20.407Z	DPANIC	kubernetes/predicates.go:123	non-string key argument passed to logging, ignoring all later arguments	{"invalid key": "not.configured/controller"}
2024-01-25T11:14:20.407Z	INFO	kubernetes/predicates.go:123	gatewayclass controller name
2024-01-25T11:14:20.407Z	INFO	kubernetes/predicates.go:124	gatewayclass name for gateway doesn't match configured name	{"namespace": "", "name": "scheduled-status-test"}
2024-01-25T11:14:20.417Z	INFO	kubernetes/routes.go:266	processing HTTPRoute	{"namespace": "test", "name": "test"}
2024-01-25T11:14:20.418Z	INFO	kubernetes/routes.go:266	processing HTTPRoute	{"namespace": "test", "name": "test"}
2024-01-25T11:14:20.418Z	INFO	kubernetes/routes.go:266	processing HTTPRoute	{"namespace": "test", "name": "test"}
2024-01-25T11:14:20.419Z	INFO	kubernetes/routes.go:137	processing GRPCRoute	{"namespace": "test", "name": "test"}

@arkodg arkodg requested a review from shawnh2 January 25, 2024 22:57
@shawnh2
Copy link
Copy Markdown
Contributor

shawnh2 commented Jan 26, 2024

/retest

shawnh2
shawnh2 reviewed Jan 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@shawnh2 shawnh2 left a comment

Choose a reason for hiding this comment

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

thanks for adding this, generally lgtm, just some nits.

@deszhou
fix lint
b48a8cd 
Signed-off-by: yeedove <yeedove@gmail.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 26, 2024

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (c86681c) 64.68% compared to head (b48a8cd) 64.62%.
Report is 6 commits behind head on main.

Files Patch % Lines
internal/provider/kubernetes/predicates.go 50.00% 4 Missing and 2 partials ⚠️
internal/provider/kubernetes/controller.go 96.15% 1 Missing ⚠️
internal/provider/kubernetes/filters.go 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2494      +/-   ##
==========================================
- Coverage   64.68%   64.62%   -0.07%     
==========================================
  Files         115      115              
  Lines       17472    17513      +41     
==========================================
+ Hits        11302    11317      +15     
- Misses       5449     5471      +22     
- Partials      721      725       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@deszhou
Copy link
Copy Markdown
Contributor Author

deszhou commented Jan 26, 2024

@shawnh2 done, thank!

shawnh2
shawnh2 approved these changes Jan 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@shawnh2 shawnh2 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

arkodg
arkodg approved these changes Jan 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks !

@arkodg arkodg merged commit e168ef9 into envoyproxy:main Jan 26, 2024
@arkodg arkodg added the release-note Indicates a required release note label Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arkodg arkodg arkodg approved these changes

@shawnh2 shawnh2 shawnh2 approved these changes

Assignees

No one assigned

Labels

release-note Indicates a required release note

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support complete label in namespaceSelectorwatch

4 participants

@deszhou @zirain @shawnh2 @arkodg