Service names #776
Service names #776
Conversation
| } | ||
| if len(executionName) > AnsibleExcecutionNameLabelLen { | ||
| executionName = executionName[:AnsibleExcecutionNameLabelLen] | ||
| } | ||
| labelValue := string(deployment.GetUID()) |
There was a problem hiding this comment.
I had continued using uid for labels to ensure that long deployment and nodeset names don't make us truncate the labels for AEE. We could have used uid for AEE names too, but we assert the names in kuttl tests (and the plan was to change those). There could be still be name collision, if the service, deployment and nodeset names are not kept small and unique.
There was a problem hiding this comment.
That's a good point. Labels need to follow RFC-952, or used to, now the documentation specifies something more like RFC-1123.
There are ways to handle that though, by setting validation for nodeset, deployment, and service name. As it happens, we already have a package for that.
Edit: I knew this sounded familiar. I've introduced that validation for label myself in 13d56de but it must have gotten lost in the meantime.
There was a problem hiding this comment.
I've added validation for nodeset, deployment and service names.
There was a problem hiding this comment.
k8s object names can be 253 chars. Limiting deployment/nodeset names to 63 chars would not achieve anything as we use a combination of deployment and nodeset name to build the label in this PR.
There was a problem hiding this comment.
Some of them do, yes. But the validation here is meant to sanitize strings we'll eventually use as labels. These are limited to 63 chars.
There was a problem hiding this comment.
|
recheck |
|
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/9d3f30d557f9492390c009c01b3b4ffc ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 45m 43s |
|
recheck |
|
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/e07a70ba7bf347278d7d71600d949f0a ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 34m 19s |
|
/retest |
|
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/9da451d45e35475abec22b1d0e59d82d ❌ openstack-k8s-operators-content-provider FAILURE in 7m 53s |
|
Hit by the go version mismatch error. |
Signed-off-by: Jiri Podivin <[email protected]>
GetAnsibleExecutionNameAndLabel now consumes names, rather than entire specs of NodeSet and deployment. Several labels were added, for nodeset, service and deployment. Signed-off-by: Jiri Podivin <[email protected]>
Signed-off-by: Jiri Podivin <[email protected]>
Since we are using names of service, nodeset and deployment to create labels for ansibleee resources we must ensure they comply with name standard[1]. Validation for nodeset name has been placed into relevant webhook, others will follow when their respective resources have webhooks implemented. Specifically, the label must conform to RFC1123. [1] https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set Signed-off-by: Jiri Podivin <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fao89, jpodivin, slagle The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/cc444cee4fb44318bfe3bec846ebec77 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 19m 09s |
|
recheck (it seems we need to bump the timeout) |
|
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/a1c671e2ed3142b68a4a40d0108dc566 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 23m 49s |
|
recheck cert_manager |
|
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/2db91e14baca40d48e9ee898616f4d05 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 53m 10s |
|
recheck cert-manager |
|
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/72f36dcf6e4c4cec8b86d444bfe4858c ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 04m 17s |
|
recheck |
openshift-merge-bot
bot
merged commit f6bc8fe
into
openstack-k8s-operators:main
| if err = validate.Var(r.Name, "hostname_rfc1123"); err != nil { | ||
| openstackdataplanenodesetlog.Error(err, "Error validating OpenStackDataPlaneNodeSet name, name must follow RFC1123") | ||
| errors = append(errors, field.Invalid( | ||
| field.NewPath("Name"), |
There was a problem hiding this comment.
In general you want to return the json path of the CRD field that failed the validation. I think in this case that is field.NewPath("metadata").Child("name").
update default after running make bundle change was not pulled in when the following pull request was merged Merge pull request openstack-k8s-operators#776 from jpodivin/service-names Signed-off-by: Jon Schlueter <[email protected]>
No description provided.