Owls 87956 - Generate shorter volume name when override secret name is too long #2257
Conversation
| .secret(getOverrideSecretVolumeSource(secretName))); | ||
| } | ||
|
|
||
| private void addConfigOverrideVolume(V1PodSpec podSpec, String configOverrides) { | ||
| podSpec.addVolumesItem( | ||
| new V1Volume() | ||
| .name(configOverrides + "-volume") | ||
| .name(configOverrides + VOLUME_NAME_SUFFIX) |
There was a problem hiding this comment.
In the spirit of fixing all of the related problems, I think we should make the same change to volume items that include the name of a config map as well. Config maps are another example of a resource that can have names longer than 63 characters.
Let's please look at PodHelper/PodStepContext too to see if we automatically generate volume/volumeMount names based on resource names.
There was a problem hiding this comment.
Done. I looked PodHelper/PodStepContext but didn't find any place where we automatically generate volume/volumeMount names based on resource names.
|
|
||
| private String getName(String secretName, AtomicInteger index) { | ||
| return secretName.length() > (MAX_ALLOWED_VOLUME_NAME_LENGTH - VOLUME_NAME_SUFFIX.length()) | ||
| ? "long-secret-name-" + index.getAndIncrement() : secretName; |
There was a problem hiding this comment.
Instead of "long-secret-name-" can we please use something simpler like "volume-" or as much of the secretName that we have room for (eliminating the end of the name).
There was a problem hiding this comment.
We suffix the volume names with "-volume" and I thought to keep the same suffix for the secrets volume names where the name exceeds 63 characters limit (for consistency). I will take the first 50 characters of the secret name and then leave the remaining characters for the ordinal/index of the volume in case there are multiple secrets having the same first 50 characters.
There was a problem hiding this comment.
And you have to make sure that modifying the domain doesn't create a different set up mappings based on the order in which the secret/volume names are shortened. One way to do that might be to compute a checksum for the original name, and then use that as part of the generated name, rather than an index.
There was a problem hiding this comment.
I have made the change to compute a checksum for the original name and use that as part of the generated name.
| @Test | ||
| public void whenDomainHasMultipleConfigOverrideSecretsWithLongNames_volumeCreatedWithShorterNames() { | ||
| resourceLookup.defineResource(LONG_SECRET_NAME, KubernetesResourceType.Secret, NS); | ||
| resourceLookup.defineResource(SECOND_LONG_SECRET_NAME, KubernetesResourceType.Secret, NS); |
There was a problem hiding this comment.
Probably want to add multiple long named secrets where the first 63 characters are the same.
…cksum and use shorter volume names for config maps.
| @@ -389,6 +394,26 @@ protected V1Container createPrimaryContainer(TuningParameters tuningParameters) | |||
| return container; | |||
| } | |||
|
|
|||
| private String getVolumeName(String resourceName, String type) { | |||
There was a problem hiding this comment.
If these methods do the same thing, why do you need two of them?
There was a problem hiding this comment.
It was leftover from the previous implementation and I thought having methods with different names might provide better readability. I went ahead and removed it.
|
|
||
| private String getShortName(String resourceName, String type) { | ||
| String volumeSuffix = VOLUME_NAME_SUFFIX + "-" + type + "-" | ||
| + Optional.ofNullable(ChecksumUtils.getMD5Hash(resourceName)).orElse(""); |
There was a problem hiding this comment.
When would the result of getMD5Hash be null?
There was a problem hiding this comment.
It can be null when MessageDigest.getInstance("MD5") method (in ChecksumUtils) throws NoSuchAlgorithmException or String.getBytes() method throws UnsupportedEncodingException.
OWLS-87956 - K8s volume name has a limit of 63 characters. For override secrets, the operator generates volume name by appending "-volume" to the secret name. If the volume name length exceeds the limit, the volume fails to be mounted.
This change creates a shorter volume name (based on the ordinal suffix) for the override secrets when the secret name exceeds the limit and uses the old naming scheme when the secret name length is less than the limit.
Added new unit tests in the JobHelperTest and integration test results are at - https://build.weblogick8s.org:8443/job/weblogic-kubernetes-operator-kind-new/4264/