Odo fails when devfile component container name is really long

[maysunfaisal]

/kind bug

What versions of software are you using?

Operating System:

Output of odo version: master

Kube has a resource name limit of 63 characters. Currently, we do not check if the devfile container component name is less than 63 characters before using it:

Maysuns-MacBook-Pro:nodejs-ex maysun$ odo push

Validation
 ✓  Validating the devfile [119014ns]

Creating Kubernetes resources for component madridnode
 ✗  Failed to start component with name madridnode. Error: Failed to create the component: unable to create or update component: unable to create Deployment madridnode: Deployment.apps "madridnode" is invalid: spec.template.spec.containers[0].name: Invalid value: "runtimeruntimeruntimeruntimeruntimeruntimeruntimeruntimeruntimeruntimeruntimeruntimeruntimeruntimeruntimeruntimeruntime": must be no more than 63 characters

Opened a new issue because it requires some code refactoring that impacts some features that rely on the name match.

/area devfile

[maysunfaisal]

One suggestion would be to use it as a map map[string]corev1.Container, rather than an array of containers []corev1.Container where the map key is the actual devfile container component name. That way it makes less of an impact with the existing code.

[yangcao77]

/assign

[yangcao77]

This issue is a little bit tricky. We have 2 options:

1. check the container name in odo push validation, to show error before the actual execution.
2. trim down the container name provided, to ~50 chars, and append 5-10 random chars at the end to ensure uniqueness

I talked with @maysunfaisal , he prefers to go for Option2. However, if we want to trim down the container name and create another unique name, we need to save the mapping of actual container name in devfile -> container name we generated for kube resources on disk; since commands/URLs would need the mapping info.

In addition, this is not a specific issue for container name; other Kube resources' names also have the limitation of 63 chars. Currently, we are not checking the length of all those resources. How do we plan to handle those resources in general?
we might want a deeper discussion on that.
ping @kadel @elsony @cdrage @girishramnani @johnmcollier @mik-dass @GeekArthur
feel free to share your thoughts

[maysunfaisal]

Thats why I opened a new issue for this because we have code in devfile that relies on container name matching and it affects a lot of code.

[elsony]

Out of the 2 options, I prefer #2 but there will be a lot of code change. If we implement #2 then we should just create a function for the name generation and all places referring to container name will need to call that. Having said that, if we want to keep things simple, we can have option 3.
3. Do component name sanitization during component creation. We'll limit the number of chars allowed for component name as well as validate to make sure the name does not contain any invalid characters (if we do not already do that). If we do it this way, it will save us from failing during the push. The problem on failing during the push operation is the user will ended up have to go back to fix it. If we know the name does not work, we should probably block it from creation.

Note: Even if we do that, we may still want to valid during the push just in case the user manually change the component name in the env.yaml after the component creation.

[yangcao77]

Out of the 2 options, I prefer #2 but there will be a lot of code change. If we implement #2 then we should just create a function for the name generation and all places referring to container name will need to call that. Having said that, if we want to keep things simple, we can have option 3.
3. Do component name sanitization during component creation. We'll limit the number of chars allowed for component name as well as validate to make sure the name does not contain any invalid characters (if we do not already do that). If we do it this way, it will save us from failing during the push. The problem on failing during the push operation is the user will ended up have to go back to fix it. If we know the name does not work, we should probably block it from creation.

Note: Even if we do that, we may still want to valid during the push just in case the user manually change the component name in the env.yaml after the component creation.

Do you mean we do validation on all resources during component creation? e.g. component name provided, container name in the devfile, endpoint name in the devfile, etc?

[GeekArthur]

@elsony @yangcao77 We already had the util function to validate the kube resource name here: https://github.com/openshift/odo/blob/master/pkg/util/util.go#L1047, it should include the validation of all kube resource name requirements (eg. number of chars, supporting chars, etc). The easiest way could be calling the util function to validate the kube resource name before generating the kube resource. Instead of doing the validation before component creation, we can do the validation before resource creation, then we won't miss any case such as user manually updates the env.yaml file after component creation.

[yangcao77]

@elsony @yangcao77 We already had the util function to validate the kube resource name here: https://github.com/openshift/odo/blob/master/pkg/util/util.go#L1047, it should include the validation of all kube resource name requirements (eg. number of chars, supporting chars, etc).

Thanks Jingfu! I'm using that function to validate the resource name.

The easiest way could be calling the util function to validate the kube resource name before generating the kube resource. Instead of doing the validation before component creation, we can do the validation before resource creation, then we won't miss any case such as user manually updates the env.yaml file after component creation.

I think the problem for this issue is we want to validate and show error before execute any operations, which saves resources and time. Validate right before resource creation also works, but it takes more effort/ did more actions for a failure case than do it inside validate() function of odo push.

[elsony]

Do you mean we do validation on all resources during component creation? e.g. component name provided, container name in the devfile, endpoint name in the devfile, etc?

Those checks are relatively quick since it is just using static rules based on the names. Therefore, if we do those validations as part of the devfile validation, then we should have all cases covered. We are calling devfile validation on both component creation as well as before the push so both scenarios should be covered.