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

Add config field for nodePlacement #2530

Merged
merged 1 commit into from
Mar 14, 2023

Conversation

vthapar
Copy link
Contributor

@vthapar vthapar commented Mar 8, 2023

@submariner-bot
Copy link
Contributor

🤖 Created branch: z_pr2530/vthapar/node-placement
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

@@ -47,6 +48,10 @@ type ServiceDiscoverySpec struct {
// +listType=set
CustomDomains []string `json:"customDomains,omitempty"`
ImageOverrides map[string]string `json:"imageOverrides,omitempty"`
// +optional
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn’t this be

Suggested change
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
NodeSelector v1.NodeSelector `json:"nodeSelector,omitempty"`

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think there is a type NodeSelector. Even PodSpec uses map[string]string

https://github.com/kubernetes/api/blob/master/core/v1/types.go#L3273

	// NodeSelector is a selector which must be true for the pod to fit on a node.
	// Selector which must match a node's labels for the pod to be scheduled on that node.
	// More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
	// +optional
	// +mapType=atomic
	NodeSelector map[string]string `json:"nodeSelector,omitempty" protobuf:"bytes,7,rep,name=nodeSelector"`

Copy link
Member

Choose a reason for hiding this comment

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

There is a NodeSelector type, hence my question: https://pkg.go.dev/k8s.io/api/core/v1#NodeSelector. I’ve seen it used in other specs, but it doesn’t really seem to be appropriate for that; let’s follow PodSpec.

@skitt skitt closed this Mar 9, 2023
@submariner-bot
Copy link
Contributor

🤖 Closed branches: [z_pr2530/vthapar/node-placement]

@skitt skitt reopened this Mar 9, 2023
@submariner-bot
Copy link
Contributor

🤖 Created branch: z_pr2530/vthapar/node-placement
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

Epic submariner-io/enhancements#149
Refer submariner-io/enhancements#156

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
@skitt skitt closed this Mar 9, 2023
@submariner-bot
Copy link
Contributor

🤖 Closed branches: [z_pr2530/vthapar/node-placement]

@skitt skitt reopened this Mar 9, 2023
@submariner-bot
Copy link
Contributor

🤖 Created branch: z_pr2530/vthapar/node-placement
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

@maayanf24 maayanf24 added this to In progress in Submariner Project 0.15 via automation Mar 13, 2023
@submariner-bot submariner-bot added the ready-to-test When a PR is ready for full E2E testing label Mar 13, 2023
@vthapar vthapar enabled auto-merge (squash) March 14, 2023 07:08
@vthapar vthapar merged commit 44897ac into submariner-io:devel Mar 14, 2023
Submariner Project 0.15 automation moved this from In progress to Done Mar 14, 2023
@submariner-bot
Copy link
Contributor

🤖 Closed branches: [z_pr2530/vthapar/node-placement]

@nyechiel nyechiel added the release-note-needed Should be mentioned in the release notes label Mar 14, 2023
@vthapar vthapar deleted the node-placement branch March 14, 2023 08:39
vthapar added a commit to vthapar/submariner-addon that referenced this pull request Apr 19, 2023
Refer submariner-io/submariner-operator#2530

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
vthapar added a commit to vthapar/submariner-addon that referenced this pull request Apr 19, 2023
Add get, list and watch permissions for AddonDeploymentConfig
so submariner addon can track changes to it.

Refer submariner-io/submariner-operator#2530

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
openshift-merge-robot pushed a commit to stolostron/submariner-addon that referenced this pull request Apr 19, 2023
Add get, list and watch permissions for AddonDeploymentConfig
so submariner addon can track changes to it.

Refer submariner-io/submariner-operator#2530

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
vthapar added a commit to vthapar/submariner-addon that referenced this pull request Apr 26, 2023
Refer submariner-io/submariner-operator#2530

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
vthapar added a commit to vthapar/submariner-addon that referenced this pull request Apr 26, 2023
Refer submariner-io/submariner-operator#2530

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
vthapar added a commit to vthapar/submariner-addon that referenced this pull request Apr 26, 2023
Refer submariner-io/submariner-operator#2530

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
vthapar added a commit to vthapar/submariner-addon that referenced this pull request Apr 26, 2023
Refer submariner-io/submariner-operator#2530

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
vthapar added a commit to vthapar/submariner-addon that referenced this pull request Apr 27, 2023
Refer submariner-io/submariner-operator#2530

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
vthapar added a commit to vthapar/submariner-addon that referenced this pull request Apr 27, 2023
Refer submariner-io/submariner-operator#2530

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
vthapar added a commit to vthapar/submariner-addon that referenced this pull request Apr 27, 2023
Refer submariner-io/submariner-operator#2530

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
vthapar added a commit to vthapar/submariner-addon that referenced this pull request Apr 27, 2023
Refer submariner-io/submariner-operator#2530

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
vthapar added a commit to vthapar/submariner-addon that referenced this pull request Apr 27, 2023
Refer submariner-io/submariner-operator#2530

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
openshift-merge-robot pushed a commit to stolostron/submariner-addon that referenced this pull request May 2, 2023
Refer submariner-io/submariner-operator#2530

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>
@dfarrell07 dfarrell07 removed the release-note-needed Should be mentioned in the release notes label May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-test When a PR is ready for full E2E testing
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants