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

implement autoBuild and deployByDefault #5694

Closed
Tracked by #5834
kadel opened this issue Apr 26, 2022 · 6 comments · Fixed by #6654
Closed
Tracked by #5834

implement autoBuild and deployByDefault #5694

kadel opened this issue Apr 26, 2022 · 6 comments · Fixed by #6654
Assignees
@rm3l
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)). sprint demo Indicates an issue for which a demo should be recorded and presented at the end of the sprint.
Milestone
v3.9.0 🚀

Comments

@kadel
Copy link
Member

kadel commented Apr 26, 2022

devfile 2.2.0 added two new fiels we need to make sure that odo has a proper support for it

devfile/api#709
@kadel kadel added priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)). area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. labels Apr 26, 2022
@kadel kadel added this to the v3.0.0-beta2 milestone May 6, 2022
@feloy
Copy link
Contributor

feloy commented Jun 2, 2022 •
edited
Loading

related: devfile/api#852

@kadel kadel mentioned this issue Jun 16, 2022
Closed
3 tasks
@dharmit
Copy link
Member

dharmit commented Jun 16, 2022

Should require making changes into https://github.com/redhat-developer/odo/blob/main/pkg/devfile/components.go#L12 as pointed by @feloy.

@kadel kadel removed this from the v3.0.0-beta2 milestone Jun 22, 2022
@feloy feloy added the triage/needs-information Indicates an issue needs more information in order to work on it. label Feb 15, 2023
@feloy
Copy link
Contributor

feloy commented Feb 15, 2023

Make research on this issue

@rm3l rm3l modified the milestones: v3.8.0 🚀, v3.9.0 🚀 Mar 1, 2023
@rm3l rm3l mentioned this issue Mar 10, 2023
Merged
3 tasks
@rm3l
Copy link
Member

rm3l commented Mar 10, 2023 •
edited
Loading

I did some research on this issue to better understand the impact on odo, based on the final proposal at devfile/api#852 (comment):

  • odo dev
  • odo deploy
  • odo build-images: currently, we are building all Image Components, regardless of whether they are referenced or not. So autoBuild won't change that behavior. We can continue to build all Images, regardless of autoBuild.
  • Binding and odo deploy: currently, odo add binding adds a Kubernetes component to the Devfile with deployByDefault not set and which is not referenced by any apply command. Per Extend deployByDefault to other components, drop the implicit apply command rule devfile/api#852 (comment), it will need to be automatically created on startup. So odo dev will auto-create it (as before). But I assume odo deploy would also do the same, which changes the behavior of odo deploy regarding ServiceBinding resources. I understand that this clarifies things, but would that be ok to change this behavior, @kadel?

I have started implementing the required changes in #6654, using this Devfile for the test cases.
And the Devfile Parser seems to currently return the default value (false) for deployByDefault and autoBuild if not set, making it quite hard to handle cases where those fields are not set, and the component is not referenced by any apply command. I'll need to investigate further..

@rm3l
Copy link
Member

rm3l commented Mar 13, 2023

I have started implementing the required changes in #6654, using this Devfile for the test cases. And the Devfile Parser seems to currently return the default value (false) for deployByDefault and autoBuild if not set, making it quite hard to handle cases where those fields are not set, and the component is not referenced by any apply command. I'll need to investigate further..

Ok, I've narrowed down the issue to a boolean passed to the Devfile parser arguments: FlattenedDevfile. If this is set to true (default value), the Parser sets all unset fields to their default values, making it hard to determine when they are not set:
https://github.com/devfile/library/blob/bd4a12f272573de1ab254f3e138a9b55904af620/pkg/devfile/parser/parse.go#L147-L153
Setting it to false (055f71a (#6654)) works fine in the scope of this issue, but creates other issues when parsing Devfiles with Parents..

@rm3l rm3l removed the triage/needs-information Indicates an issue needs more information in order to work on it. label Mar 20, 2023
@rm3l rm3l mentioned this issue Mar 21, 2023
Closed
@rm3l rm3l added the status/blocked Denotes an issue or PR that is blocked on something (e.g., issue/PR in different repo) label Mar 21, 2023
@rm3l
Copy link
Member

rm3l commented Mar 21, 2023

Blocked by devfile/api#1067

@michael-valdron michael-valdron mentioned this issue Mar 24, 2023
Open
2 tasks
@rm3l rm3l added the sprint demo Indicates an issue for which a demo should be recorded and presented at the end of the sprint. label Mar 29, 2023
rm3l added a commit to rm3l/odo that referenced this issue Mar 31, 2023
@rm3l
Do not set default values when parsing a Devfile
04052e5 
See [1] for the rationale

[1] redhat-developer#5694
rm3l added a commit to rm3l/odo that referenced this issue Apr 4, 2023
@rm3l
Do not set default values when parsing a Devfile
0760872 
See [1] for the rationale

[1] redhat-developer#5694
@rm3l rm3l removed the status/blocked Denotes an issue or PR that is blocked on something (e.g., issue/PR in different repo) label Apr 4, 2023
openshift-merge-robot pushed a commit that referenced this issue Apr 5, 2023
@rm3l @feloy @valaparthvi
Support autoBuild and deployByDefault on Image and Kubernetes/Ope…
cc2e8de 
…nShift components (#6654)

* Add sample Devfile with multiple autoBuild/deployByDefault combinations

It will be used for integration tests.

* Add helper function to update a given Devfile Command Group

This will allow supporting cases where we need to run a custom command,
but this is not implemented yet on odo (cases with 'odo dev --debug'
and 'odo deploy').
In this case, this helper will allow updating the Devfile for example to
unmark the current default command as non-default, and mark the custom
one as default.

* Add test cases for 'odo dev'

* Add test cases for 'odo deploy'

* Add test cases for 'odo build-images'

'odo build-images' should build all images regardless of the 'autoBuild' property.

* Display the spinner when creating or updating Kubernetes resources

This helps understand what resources are being patched.

* Handle deployByDefault on K8s and OpenShift components

* Add 'devfile.GetImageComponentsToPush' functions that returns the list of image components to auto-create

See devfile/api#852 (comment) for more context.

* Handle automatic image component creation for 'odo dev' on Kubernetes

* Handle automatic image component creation for 'odo dev' on Podman

* Handle automatic image and K8s/OpenShift component creation for 'odo deploy'

* Bump Devfile library to the latest commit at this time (c1b23d2)

This includes the fix for [1], which provides a way not to set default values automatically

[1] devfile/api#1067

* Do not set default values when parsing a Devfile

See [1] for the rationale

[1] #5694

* Add documentation in the Devfile reference page

* Revert "Display the spinner when creating or updating Kubernetes resources"

This reverts commit 6ad073e63cb0e685f165eed767619a90997393a3.

* Avoid re-applying Image components multiple times

'adapter#Push' might get called several times when there are
state transitions (either on the Deployment in the cluster,
or from 'odo').
It might be confusing to apply Image components over and over again
(and also it can be slower if we need to push images to remote registries).

* Move GetK8sAndOcComponentsToPush and GetImageComponentsToPush to libdevfile package

As suggested in review, this should be the responsibility of the devfile library

Co-authored-by: Philippe Martin <phmartin@redhat.com>

* fixup! Handle automatic image and K8s/OpenShift component creation for 'odo deploy'

* fixup! Handle automatic image component creation for 'odo dev' on Podman

* fixup! Avoid re-applying Image components multiple times

* Apply suggestions from code review

Co-authored-by: Parthvi Vala <pvala@redhat.com>

* fixup! Do not set default values when parsing a Devfile

* Fix devfile-deploy-functional-pods.yaml (used in 'odo logs' tests)

Set deployByDefault to false on innerloop-pod component.
Otherwise, since it is not referenced by any apply command,
it will be automatically created by *both* 'odo dev' and 'odo deploy'.

---------

Co-authored-by: Philippe Martin <phmartin@redhat.com>
Co-authored-by: Parthvi Vala <pvala@redhat.com>
@rm3l rm3l mentioned this issue Apr 5, 2023
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rm3l rm3l

Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)). sprint demo Indicates an issue for which a demo should be recorded and presented at the end of the sprint.
Projects
odo Project
Archived in project
after v3.0.0
Status: Done
Milestone
v3.9.0 🚀
Development

Successfully merging a pull request may close this issue.

Support autoBuild and deployByDefault on Image and Kubernetes/OpenShift components rm3l/odo
4 participants
@kadel @rm3l @dharmit @feloy