-
Notifications
You must be signed in to change notification settings - Fork 66
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
openscapes: update EKS cluster config templates from k8s 1.21 to 1.24 #2139
openscapes: update EKS cluster config templates from k8s 1.21 to 1.24 #2139
Conversation
/* | ||
This file is a jsonnet template of a eksctl's cluster configuration file, | ||
that is used with the eksctl CLI to both update and initialize an AWS EKS | ||
based cluster. | ||
|
||
This file has in turn been generated from eksctl/template.jsonnet which is | ||
relevant to compare with for changes over time. | ||
|
||
To use jsonnet to generate an eksctl configuration file from this, do: | ||
|
||
jsonnet << cluster_name >>.jsonnet > eksctl-config.yaml | ||
|
||
References: | ||
- https://eksctl.io/usage/schema/ | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a jsonnet comment to be retained after being generated from the jinja2 template, while the {#
parts above are jinja2 comments just for this template.jsonnet file.
f6ea7dd
to
bb15478
Compare
|
||
// Node definitions for dask worker nodes. Config here is merged | ||
// with our dask worker node definition, which uses spot instances. | ||
// A `node.kubernetes.io/instance-type label is set to the name of the | ||
// *first* item in instanceDistribution.instanceTypes, to match | ||
// what we do with notebook nodes. Pods can request a particular | ||
// kind of node with a nodeSelector | ||
local daskNodes = [ | ||
// Node definitions for dask worker nodes. Config here is merged | ||
// with our dask worker node definition, which uses spot instances. | ||
// A `node.kubernetes.io/instance-type label is set to the name of the | ||
// *first* item in instanceDistribution.instanceTypes, to match | ||
// what we do with notebook nodes. Pods can request a particular | ||
// kind of node with a nodeSelector |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a relocation of these comments, matching how they now generate from the template.jsonnet file as changed in another unrelated PR.
nodeGroups: [n + {clusterName:: $.metadata.name} for n in [ | ||
ng { | ||
name: 'core-b', | ||
name: 'core-a', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During this upgrade, I deleted the core-b
nodepool and added the core-a
nodepool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me. Thanks @consideRatio
Also, thanks for the inline explanations. Super useful
This updates the eksctl configuration template files for the openscapes cluster that I've migrated from k8s 1.21 to k8s 1.24 now.
If you want me to re-formulate something, I'd like to do it in a standalone PR after this and #2149 is merged which also includes the same comments copied to many files.