Skip to content

Add node affinity support #1166

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

Merged
merged 23 commits into from
Dec 16, 2020
Merged

Add node affinity support #1166

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f842529
Merge pull request #1 from zalando/master
sagor999 Oct 10, 2020
abc2cb4
Merge pull request #3 from zalando/master
sagor999 Nov 17, 2020
d412754
Merge pull request #4 from zalando/master
sagor999 Dec 6, 2020
abc48a6
Merge pull request #5 from zalando/master
sagor999 Dec 15, 2020
c7aadba
Adding nodeaffinity support alongside node_readiness_label
spohner May 13, 2020
4014442
Add nodeAffinity support to ConnectionPooler
adastleyatvi Jul 3, 2020
b6a83ca
Add tests for ConnectionPooler nodeAffinity
adastleyatvi Oct 10, 2020
9b53763
fix compareStatefulSet to take into account nodeAffinity
sagor999 Oct 11, 2020
b8751e8
add documentation for node affinity
sagor999 Oct 12, 2020
58c2142
add node affinity test
sagor999 Oct 12, 2020
300183f
fix print statement
sagor999 Oct 18, 2020
04be950
update codegen
sagor999 Oct 20, 2020
a3dea03
fix crd api for node affinity
sagor999 Nov 17, 2020
f032cd5
fix node affinity test
sagor999 Nov 17, 2020
fc9d5bf
fix e2e test for node affinity
sagor999 Nov 18, 2020
88743f8
replace hardcoded sleep with wait_for_pod_start in node affinity e2e
sagor999 Nov 18, 2020
a8c9b0e
remove extra affinity check, as it is already done
sagor999 Dec 6, 2020
8f05753
improve crd readability by moving Required field up
sagor999 Dec 6, 2020
03f91c3
improve node affinity e2e test
sagor999 Dec 6, 2020
867b294
add node affinity into various crds and add example into complete man…
sagor999 Dec 6, 2020
3bfa3af
fix missing type in crd
sagor999 Dec 6, 2020
8e8cd93
remove node affinity from connection pooler
sagor999 Dec 15, 2020
f5a871b
fix unit test for node affinity
sagor999 Dec 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add nodeAffinity support to ConnectionPooler
  • Loading branch information
@adastleyatvi @sagor999
adastleyatvi authored and sagor999 committed Dec 15, 2020
commit 40144428ef71b837590743175c33b326ffb25937
8 changes: 7 additions & 1 deletion pkg/apis/acid.zalan.do/v1/crds.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,12 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{
"user": {
Type: "string",
},
"nodeAffinity": {
Copy link
Member

@FxKu FxKu Dec 1, 2020

Choose a reason for hiding this comment

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

gosh, that's a lot of lines. Is this validation covering all affinity options? Maybe we just cut it short with:

"nodeAffinity": {
	Type:                   "object",
	XPreserveUnknownFields: util.True(),
},

Copy link
Member

@FxKu FxKu Dec 1, 2020

Choose a reason for hiding this comment

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

at least, makes it easier to copy it to the CRD yaml manifest where this part is still missing, I think

Copy link
Contributor Author

@sagor999 sagor999 Dec 2, 2020

Choose a reason for hiding this comment

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

@FxKu for some reason, it was just kept trimming extra fields, even with XPreserveUnknownFields. That is why I had to expand it almost fully. :(

Copy link
Contributor Author

@sagor999 sagor999 Dec 6, 2020

Choose a reason for hiding this comment

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

I just tested it again, and yes, even with XPreserveUnknownFields it still trims unknown fields and completely breaks node affinity support. Btw, before Nov 17th, it used to work fine.

Copy link
Member

@FxKu FxKu Dec 15, 2020

Choose a reason for hiding this comment

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

ok, can you remove this part from the validation as it covers the anti_affinity of the pooler

Copy link
Contributor Author

@sagor999 sagor999 Dec 15, 2020

Choose a reason for hiding this comment

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

sounds good! will remove

Type: "object",
AdditionalProperties: &apiextv1beta1.JSONSchemaPropsOrBool{
Allows: true,
},
},
},
},
"databases": {
Expand Down Expand Up @@ -597,7 +603,7 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{
},
},
},
"nodeaffinity": {
"nodeAffinity": {
Type: "object",
AdditionalProperties: &apiextv1beta1.JSONSchemaPropsOrBool{
Allows: true,
Expand Down
13 changes: 7 additions & 6 deletions pkg/apis/acid.zalan.do/v1/postgresql_type.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,12 +214,13 @@ type PostgresStatus struct {
// makes sense to expose. E.g. pool size (min/max boundaries), max client
// connections etc.
type ConnectionPooler struct {
NumberOfInstances *int32 `json:"numberOfInstances,omitempty"`
Schema string `json:"schema,omitempty"`
User string `json:"user,omitempty"`
Mode string `json:"mode,omitempty"`
DockerImage string `json:"dockerImage,omitempty"`
MaxDBConnections *int32 `json:"maxDBConnections,omitempty"`
NumberOfInstances *int32 `json:"numberOfInstances,omitempty"`
Schema string `json:"schema,omitempty"`
User string `json:"user,omitempty"`
Mode string `json:"mode,omitempty"`
DockerImage string `json:"dockerImage,omitempty"`
MaxDBConnections *int32 `json:"maxDBConnections,omitempty"`
NodeAffinity v1.NodeAffinity `json:"nodeAffinity,omitempty"`
Copy link
Member

@FxKu FxKu Dec 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Do we need a NodeAffinity setting for the pooler, too? @erthalion @Jan-M
Can we not reuse the affinity defined in the Postgresql spec?
If I see it correctly this field isn't even reflected on creation and syncing, e.g. it is not set in generateConnectionPoolerPodTemplate. Maybe we leave it out for this PR?

Copy link
Member

@FxKu FxKu Dec 15, 2020

Choose a reason for hiding this comment

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

Can you remove the field NodeAffintity from pooler API?

Copy link
Contributor Author

@sagor999 sagor999 Dec 15, 2020

Choose a reason for hiding this comment

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

Will do!


Resources `json:"resources,omitempty"`
}