console
console copied to clipboard
Update Devfile Import to use Kubernetes YAML definitions
Description
This PR updates the Devfile handler backend code to consume the Kubernetes YAML file definitions. For now, the devfile samples only have the Deployment definition in the YAML file but the next course of action would be to update the Service, Route definitions in the devfile sample
Fixes https://github.com/devfile/api/issues/924
This PR just added the ability to read from Kubernetes YAML if the Devfile Kubernetes Component has inlined definition. If inlined is not present, it resorts to the previous way of generating Kubernetes resources. This should be compatible with previous OpenShift versions and the master branch
This PR also refactored some of the devfile backend code to keep it more clean and organized. Added tests to the devfile pkg since ODC didnt have go tests. Coverage %:
How to test
(Check history for prev edit)
- Devfile Import should work fine from the UI
- If you are interested in testing the new approach, go to devfile-handler.go and update the following lines. Build and test it out by only importing a python devfile sample
// Get devfile content and parse it using a library call in the future
// devfileContentBytes := []byte(data.Devfile.DevfileContent)
convert := true
devfileObj, _, err = devfile.ParseDevfileAndValidate(parser.ParserArgs{
// Data: devfileContentBytes,
URL: "https://raw.githubusercontent.com/devfile-samples/devfile-sample-python-basic/87e269d5e018c904ba0a72b53292b3a9ed0b8b37/devfile.yaml",
ConvertKubernetesContentInUri: &convert,
})
@yangcao77 could you PTAL from Devfile perspective?
@jerolimov Thanks for the detailed testing and reports. The routes need to be defined in the devfile this way https://github.com/devfile-samples/devfile-sample-code-with-quarkus/pull/7/files#diff-23d39e3ae288b080d8aa0995928cd3c432f887a75f0ab3bba675089f51a4af8eR36-R39
One could also define routes in the YAML files but our official devfile samples will have the route definition in the component[x].kubernetes.endpoints
@jerolimov Thanks for the detailed testing and reports. The routes need to be defined in the devfile this way https://github.com/devfile-samples/devfile-sample-code-with-quarkus/pull/7/files#diff-23d39e3ae288b080d8aa0995928cd3c432f887a75f0ab3bba675089f51a4af8eR36-R39
One could also define routes in the YAML files but our official devfile samples will have the route definition in the component[x].kubernetes.endpoints
Hi @maysunfaisal, I understand that it should be defined somewhere. But when its not defined the backend should not create broken resource definitions. I suggest that the Route is not exported in this case. Can you convert it to a pointer so that the JSON doesn't contain it anymore in this case?
@jerolimov Thanks for the detailed testing and reports. The routes need to be defined in the devfile this way https://github.com/devfile-samples/devfile-sample-code-with-quarkus/pull/7/files#diff-23d39e3ae288b080d8aa0995928cd3c432f887a75f0ab3bba675089f51a4af8eR36-R39 One could also define routes in the YAML files but our official devfile samples will have the route definition in the component[x].kubernetes.endpoints
Hi @maysunfaisal, I understand that it should be defined somewhere. But when its not defined the backend should not create broken resource definitions. I suggest that the
Routeis not exported in this case. Can you convert it to a pointer so that the JSON doesn't contain it anymore in this case?
@maysunfaisal I agree with @jerolimov here that backend should not return a broken resource definition since the add flows will break. In case the route is not defined let's not return it. Please update the PR accordingly.
@maysunfaisal I tested the UI PR with the current backend PR and it seems to work fine expect for the issue mentioned above. Resource URI is getting replaced with inline resource yaml and the api call is also successful with the new payload. All the desired resources are also returned but the route resource is broken as mentioned above.
https://user-images.githubusercontent.com/20724543/196163901-7499a168-7a35-4fe4-9ced-a139e584534c.mov
cc: @jerolimov @invincibleJai
@divyanshiGupta @jerolimov I have rebased and addressed the PR concerns as well as our internal Slack thread conversation. PTAL.
I have tested 3 cases:
- Importing devfile sample without inline, it falls back to the current master branch approach. Successful.
- Importing devfile sample with inline including deployment, service and endpoint. Successful.
- Importing devfile sample with inline with only deployment, where service and endpoint are missing. Errors Out:
I observed one thing we should check on Slack. The Deployment contains a "variable" "{{CONTAINER_IMAGE}}" as image container.
@maysunfaisal It seems that this should be replaced with the image-name in the devfile. As @jerolimov mentioned its fine for phase 1 but lets improve it later and make sure all variables are replaced with their respective values.
/label docs-approved /label px-approved
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: jerolimov, maysunfaisal
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [jerolimov]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Verified on cluster created on pr
/label qe-approved
/retest
@maysunfaisal: all tests passed!
Full PR test history. Your PR dashboard.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.