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.

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

Jump to bottom

Fix the taints and labels #70

Merged
merged 1 commit into from
May 21, 2024
Merged

Fix the taints and labels #70

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion api/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ SHELL = /bin/bash
generate:
tsp format --check "**/*.tsp"
tsp compile redhatopenshift/HcpCluster --warn-as-error
autorest autorest-config.yaml
autorest --verbose autorest-config.yaml

.PHONY: fmt
fmt:
Expand Down
65 changes: 62 additions & 3 deletions api/redhatopenshift/HcpCluster/hcpCluster-models.tsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,41 @@ model NodePoolProperties {
spec: NodePoolSpec;
}

/** taintKey is the k8s valid key of the taint type on the nodepool nodes
* The good example of the taint key is `node-role.kubernetes.io/master`
*/
@minLength(1)
@maxLength(316)
scalar taintKey extends string;

/** taintValue is the k8s valid value of the taint type on the nodepool nodes
* The good example of the taint value is `NoSchedule`
*/
@minLength(1)
@maxLength(63)
scalar taintValue extends string;

/** labelValue is the k8s valid value of the label on the nodepool nodes
* The good example of the label value is `master`
*/
@minLength(1)
@maxLength(63)
scalar labelValue extends string;
Comment on lines +443 to +462
Copy link
Contributor

Choose a reason for hiding this comment

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

Will we need a regex restriction here? https://kubernetes.io/docs/reference/kubectl/generated/kubectl_taint/

  • The key must begin with a letter or number, and may contain letters, numbers, hyphens, dots, and underscores, up to 253 characters.
  • Optionally, the key can begin with a DNS subdomain prefix and a single '/', like example.com/my-app.
  • The value is optional. If given, it must begin with a letter or number, and may contain letters, numbers, hyphens, dots, and underscores, up to 63 characters.

Copy link
Member Author

Choose a reason for hiding this comment

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

We talked about this with @miguelsorianod. We will validate later on the same as is done internally in k8s. Originally I have written the regex, but covering all edge cases made that regex non maintainable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I'm ok with that - how do we normally keep track of that, a TODO comment or follow up card?


/** The taint effect the same as in K8s */
union Effect {
string,

/** NoSchedule taint effect */
NoSchedule: "NoSchedule",

/** PreferNoSchedule taint effect */
PreferNoSchedule: "PreferNoSchedule",

/** NoExecute taint effect */
NoExecute: "NoExecute",
}

/** Worker node pool profile */
model NodePoolSpec {
/** OpenShift version for the nodepool */
Expand All @@ -461,13 +496,19 @@ model NodePoolSpec {
/** Representation of a autoscaling in a node pool. */
autoScaling?: NodePoolAutoScaling;

/** Labels for the nodes */
// This warning means to ensure good customer experience, to not create
// poorly defined types. However here the type literarly is the map<string, value>
/** K8s labels to propagate to the NodePool Nodes
* The good example of the label is `node-role.kubernetes.io/master: ""`
*/
#suppress "@azure-tools/typespec-azure-resource-manager/arm-no-record" ""
@visibility("create", "update")
labels?: string[];
labels?: Record<labelValue>;

/** Taints for the nodes */
@visibility("create", "update")
taints?: string[];
@OpenAPI.extension("x-ms-identifiers", ["key", "value", "effect"])
taints?: Taint[];

/*
* The Tuned API is defined here:
Expand All @@ -486,6 +527,24 @@ model NodePoolSpec {
tuningConfigs?: string[];
}

/** Taint is controlling the node taint and its effects */
model Taint {
petrkotas marked this conversation as resolved.
Show resolved Hide resolved
/** The key of the taint
* The good example of the taint key is `node-role.kubernetes.io/master`
*/
key: taintKey;

/** The value of the taint
* The good example of the taint value is `NoSchedule`
*/
value?: taintValue;

/** The effect of the taint
* The good example of the taint effect is `NoSchedule`
*/
effect: Effect;
}

/** Azure node pool platform configuration */
model NodePoolPlatformProfile {
/** The resourceId for the subnet used by the workers */
Expand Down
100 changes: 90 additions & 10 deletions ...nshift/resource-manager/Microsoft.RedHatOpenshift/preview/2024-06-10-preview/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1192,6 +1192,36 @@
"type": "object",
"description": "DNS contains the DNS settings of the cluster"
},
"Effect": {
"type": "string",
"description": "The taint effect the same as in K8s",
"enum": [
"NoSchedule",
"PreferNoSchedule",
"NoExecute"
],
"x-ms-enum": {
"name": "Effect",
"modelAsString": true,
"values": [
{
"name": "NoSchedule",
"value": "NoSchedule",
"description": "NoSchedule taint effect"
},
{
"name": "PreferNoSchedule",
"value": "PreferNoSchedule",
"description": "PreferNoSchedule taint effect"
},
{
"name": "NoExecute",
"value": "NoExecute",
"description": "NoExecute taint effect"
}
]
}
},
"ExternalAuthClaimProfile": {
"type": "object",
"description": "External auth claim profile",
Expand Down Expand Up @@ -1446,10 +1476,10 @@
"description": "Representation of a autoscaling in a node pool."
},
"labels": {
"type": "array",
"description": "Labels for the nodes",
"items": {
"type": "string"
"type": "object",
"description": "K8s labels to propagate to the NodePool Nodes\nThe good example of the label is `node-role.kubernetes.io/master: \"\"`",
"additionalProperties": {
"$ref": "#/definitions/labelValue"
},
"x-ms-mutability": [
"update",
Expand All @@ -1460,8 +1490,13 @@
"type": "array",
"description": "Taints for the nodes",
"items": {
"type": "string"
"$ref": "#/definitions/Taint"
},
"x-ms-identifiers": [
"key",
"value",
"effect"
],
"x-ms-mutability": [
"update",
"create"
Expand Down Expand Up @@ -1901,10 +1936,10 @@
"description": "Representation of a autoscaling in a node pool."
},
"labels": {
"type": "array",
"description": "Labels for the nodes",
"items": {
"type": "string"
"type": "object",
"description": "K8s labels to propagate to the NodePool Nodes\nThe good example of the label is `node-role.kubernetes.io/master: \"\"`",
"additionalProperties": {
"$ref": "#/definitions/labelValue"
},
"x-ms-mutability": [
"update",
Expand All @@ -1915,8 +1950,13 @@
"type": "array",
"description": "Taints for the nodes",
"items": {
"type": "string"
"$ref": "#/definitions/Taint"
},
"x-ms-identifiers": [
"key",
"value",
"effect"
],
"x-ms-mutability": [
"update",
"create"
Expand Down Expand Up @@ -2052,6 +2092,28 @@
}
}
},
"Taint": {
"type": "object",
"description": "Taint is controlling the node taint and its effects",
"properties": {
"key": {
"$ref": "#/definitions/taintKey",
"description": "The key of the taint\nThe good example of the taint key is `node-role.kubernetes.io/master`"
},
"value": {
"$ref": "#/definitions/taintValue",
"description": "The value of the taint\nThe good example of the taint value is `NoSchedule`"
},
"effect": {
"$ref": "#/definitions/Effect",
"description": "The effect of the taint\nThe good example of the taint effect is `NoSchedule`"
}
},
"required": [
"key",
"effect"
]
},
"TokenClaimMappingsProfile": {
"type": "object",
"description": "External auth claim mappings profile",
Expand Down Expand Up @@ -2185,6 +2247,24 @@
}
]
}
},
"labelValue": {
"type": "string",
"description": "labelValue is the k8s valid value of the label on the nodepool nodes\nThe good example of the label value is `master`",
"minLength": 1,
"maxLength": 63
},
"taintKey": {
"type": "string",
"description": "taintKey is the k8s valid key of the taint type on the nodepool nodes\nThe good example of the taint key is `node-role.kubernetes.io/master`",
"minLength": 1,
"maxLength": 316
},
"taintValue": {
"type": "string",
"description": "taintValue is the k8s valid value of the taint type on the nodepool nodes\nThe good example of the taint value is `NoSchedule`",
"minLength": 1,
"maxLength": 63
}
},
"parameters": {}
Expand Down
21 changes: 21 additions & 0 deletions internal/api/v20240610preview/generated/constants.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 18 additions & 6 deletions internal/api/v20240610preview/generated/models.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

35 changes: 35 additions & 0 deletions internal/api/v20240610preview/generated/models_serde.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions internal/api/v20240610preview/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,5 @@ func init() {
validate.RegisterAlias("enum_provisioningstate", EnumValidateTag(generated.PossibleProvisioningStateValues()...))
validate.RegisterAlias("enum_resourceprovisioningstate", EnumValidateTag(generated.PossibleResourceProvisioningStateValues()...))
validate.RegisterAlias("enum_visibility", EnumValidateTag(generated.PossibleVisibilityValues()...))
validate.RegisterAlias("enum_effect", EnumValidateTag(generated.PossibleEffectValues()...))
}
Loading