Skip to content

Mikolaj/add custom node config 2023 06 02 preview #24539

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
FumingZhang merged 5 commits into from
Jun 25, 2023
Merged

Mikolaj/add custom node config 2023 06 02 preview #24539

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2a64ff3
feat: add Ulimit config and updated sysctl ranges to swagger
Jun 21, 2023
21db7ac
fix: use ulimits in json of LinuxOSConfig
Jun 21, 2023
3502f2b
feat: add response example
Jun 21, 2023
9496737
fix: add ulimits to custom words
Jun 21, 2023
7d074d8
Revert "fix: add ulimits to custom words"
Jun 25, 2023
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
3 changes: 2 additions & 1 deletion custom-words.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2844,4 +2844,5 @@ manytomany
stransparent
forceencryption
tlsciphers
tlsprotocols
tlsprotocols
ulimits

@FumingZhang FumingZhang Jun 25, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please create a separate PR to modify this file (custom-words.txt), as this is a common file, it's likely to bring merge conflict.

12 changes: 10 additions & 2 deletions ...nerService/aks/preview/2023-06-02-preview/examples/AgentPoolsCreate_CustomNodeConfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@
},
"transparentHugePageEnabled": "always",
"transparentHugePageDefrag": "madvise",
"swapFileSizeMB": 1500
"swapFileSizeMB": 1500,
"ulimits": {
"maxLockedMemory": "infinity",
"noFile": "1000"
}
}
}
}
Expand Down Expand Up @@ -74,7 +78,11 @@
},
"transparentHugePageEnabled": "always",
"transparentHugePageDefrag": "madvise",
"swapFileSizeMB": 1500
"swapFileSizeMB": 1500,
"ulimits": {
"noFile": "1000",
"maxLockedMemory": "infinity"
}
}
}
}
Expand Down
24 changes: 24 additions & 0 deletions ...ce-manager/Microsoft.ContainerService/aks/preview/2023-06-02-preview/managedClusters.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6522,6 +6522,10 @@
"type": "integer",
"format": "int32",
"description": "The size in MB of a swap file that will be created on each node."
},
"ulimits": {
"$ref": "#/definitions/UlimitConfig",
"description": "Ulimit settings for Linux agent nodes."
}
}
},
Expand Down Expand Up @@ -6635,6 +6639,8 @@
"netIpv4TcpkeepaliveIntvl": {
"type": "integer",
"format": "int32",
"minimum": 10,
"maximum": 90,
"description": "Sysctl setting net.ipv4.tcp_keepalive_intvl."
},
"netIpv4TcpTwReuse": {
Expand Down Expand Up @@ -6663,11 +6669,15 @@
"netNetfilterNfConntrackMax": {
"type": "integer",
"format": "int32",
"minimum": 131072,

@UtheMan UtheMan Jun 21, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

All the changes for minimum/maximum were not there before, hence they are getting reported as broken changes (even though new ranges are bigger than the limits we enforced before)

@UtheMan UtheMan Jun 21, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We could also remove these range changes to avoid breaking errors, no preference.

@matthchr matthchr Jun 22, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consult with @FumingZhang, but IMO it's OK to add these as they are bigger than the old actual limits enforced by the old RP. Also happy to remove if we don't want to deal with breaking change pain it adds, but if we do decide to remove we should consider putting it into the documentation manually what the min/max values are.

@UtheMan UtheMan Jun 22, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There will be more changes to custom node config in the coming months, I could add all the limitations to swagger then, to avoid adding it only to select few parameters.

So now I'd remove them for easier merge of those changes and re-add them with new changes in the coming months, thoughts? @matthchr @FumingZhang

@matthchr matthchr Jun 22, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should in theory be able to get this merged without too much issue as is, IMO. But really defer to what @FumingZhang thinks/wants here. My 2c is keep them.

@UtheMan UtheMan Jun 22, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks Matthew! Will wait for Fuming's opinion.

@FumingZhang FumingZhang Jun 25, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's ok to have such a breaking change, will go through the breaking change review once I create the PR based on the dev branch later.

"maximum": 2097152,
"description": "Sysctl setting net.netfilter.nf_conntrack_max."
},
"netNetfilterNfConntrackBuckets": {
"type": "integer",
"format": "int32",
"minimum": 65536,
"maximum": 524288,
"description": "Sysctl setting net.netfilter.nf_conntrack_buckets."
},
"fsInotifyMaxUserWatches": {
Expand Down Expand Up @@ -6712,6 +6722,20 @@
}
}
},
"UlimitConfig": {
"description": "Ulimit settings for Linux agent nodes",
"type": "object",
"properties": {
"maxLockedMemory": {
"type": "string",
"description": "Maximum locked-in-memory address space (KB)"
},
"noFile": {
Comment thread
matthchr marked this conversation as resolved.
"type": "string",
"description": "Maximum number of open files"
}
}
},
"ManagedClusterHTTPProxyConfig": {
"description": "Cluster HTTP proxy configuration.",
"type": "object",
Expand Down