-
Notifications
You must be signed in to change notification settings - Fork 243
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.
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
Lifecycle events proposal #3413
Changes from 9 commits
9d94c00
408cfce
6337c26
e34c98f
cd1ac35
9e7e394
c746a9e
51f5643
ba95914
f44bba0
94a539a
2669c6d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
# Support execution of the commands based on lifecycle events | ||
|
||
## Abstract | ||
Devfile 2.0.0 allows binding of commands to specific lifecycle events: https://github.com/devfile/kubernetes-api/issues/32. These commands are meant to be executed when those specific lifecycle events are triggered. | ||
|
||
The lifecycle events that have been implemented in Devfile 2.0.0 are generic. Their effect/meaning might be slightly different in the context a workspace (Che) or a single project (odo). However, odo should aim to provide a similar user experience within the scope of a single project. | ||
|
||
## Motivation | ||
Devfiles support commands that can be triggered based on development lifecycle events. With the implementation of lifecycle events, stack creators will be able to leverage all the capabilities of devfiles when building the correct experience/features for their stacks. `odo` should support/execute these commands at appropriate times within `odo` development flow. | ||
|
||
## User Stories | ||
Each lifecycle event would be suited to individual user stories. We propose that at least one issue is made for each of them for tracking and implementation purposes. | ||
|
||
- Add support for `postStart` commands: [container initialization support](https://github.com/openshift/odo/issues/2936) | ||
- Add support for `preStop` commands: | ||
|
||
## Design overview | ||
Some of the events might not be useful for `odo` to adopt. We should only support the ones that have a clear use-case, and then add more as other use-cases emerge. | ||
|
||
For odo, the most important events are the `postStart`, and the `preStop` because they have clear, reasonable use case within odo’s flow. There is no clear use case within odo for `preStart` and `postStop` today. | ||
|
||
- `preStart` - Do not support in odo | ||
- `postStart` - support executing these commands as part of component initialization | ||
- `preStop` - support executing these commands as part of the component delete | ||
- `postStop` - Do not support in odo | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that this can be supported in odo. It can simply execute after There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can support @kadel are you recommending that we support both and do the same thing for them effectively? It might be ok for completeness but I feel that might be more confusing for devfile creators. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It already got into devfile spec, so someone probably has a use case for it.
I don't see any problems with this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not clear on how we want to implement this. If someone specify a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see your point on partial implementation. However, if we consider stop is equivalent to deleting the component and containers, the flow would be:
The issue now is that by the time we have done the stop/delete operation, we don't have any existing deployment where we can execute the the If I change this to what you are proposing:
Then we really are changing the meaning of postStop and I would be concerned about that. I feel the real use case for odo could potentially come from actions that need to be take on "local" target. As in you want to clean up some temporary files from the developer's workspace on the host. That could be a good fit, but the way to execute them will be different. than running them on the cluster, and I'm not sure if odo supports that today. Implementing this will be trivial as we just need to parse few more commands, but I'm trying to get clarity on use case before we go down this path. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That is true. This can be fairly easily implemented. We can just create standalone Pod or Job after the component is deleted. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've tried to re-read : devfile/api#32 and found something that I had missed earlier:
There is a clear requirement that container used for preStart and postStart commands will not overlap with containers of the main workspace pod. This makes is a bit clearer and perhaps we can implement postStop as:
or change the meaning slightly and:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What would we be deleting with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can update the proposal to use a standalone pod for I'll reach out to the devfiles team to get more clarity on the use case they had in mind too. |
||
|
||
As per the Devfile 2.0.0 design specification: | ||
- Commands associated with a lifecycle binding should provide a mechanism to figure out when they have completed. That means that the command should be terminating or having a readiness probe. | ||
|
||
- Commands associated with the same lifecycle binding do not guarantee to be executed sequentially or in any order. To run commands in a given order a user should use a composite. | ||
|
||
The flow for initializing and deleting components should be updated to add execution of commands bound to relevant lifecycle event while initializing and destroying any containers. | ||
|
||
### Initialize containers | ||
- Identify which containers have been created as part of creating/updating the component deployment | ||
- Initial implementation can assume containers are only initialised during the component creation (first push). We can improve this in future to allow execution of postStart commands at a more granular level. | ||
- Sync the source code to the containers | ||
- Iterate over the list of `postStart` commands and identify the ones that are associated with newly started containers | ||
upLukeWeston marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- Execute the identified commands one by one | ||
- execute the command in the container | ||
- on failure, display a meaningful error message and stop | ||
- on success, execute next command in the list (if any) | ||
- Consider the component has been successfully initialised and continue as usual (e.g. execute build command) | ||
|
||
Note: This functionality would act as a replacement for `devInit` in devfile v1.0. | ||
|
||
### Destroy containers | ||
- Identify which containers will be deleted as of a result of deleting the component deployment | ||
- Prepare a list of `preStop` commands that are associated with the containers being destroyed | ||
- Execute the preStop commands one by one | ||
- execute the command in the container | ||
- on failure, display a meaningful error message and stop | ||
- on success, execute next command in the list (if any) | ||
- Destroy/delete the component and continue as usual | ||
|
||
## Example devfile with lifecycle binding: | ||
``` | ||
schemaVersion: "2.0.0" | ||
metadata: | ||
name: test-devfile | ||
projects: | ||
- name: nodejs-web-app | ||
git: | ||
location: "https://github.com/che-samples/web-nodejs-sample.git" | ||
components: | ||
- container: | ||
id: tools | ||
image: quay.io/eclipse/che-nodejs10-ubi:nightly | ||
name: "tools" | ||
- container: | ||
id: runtime | ||
image: quay.io/eclipse/che-nodejs10-ubi:nightly | ||
name: "runtime" | ||
commands: | ||
- exec: | ||
id: download dependencies | ||
commandLine: "npm install" | ||
component: tools | ||
workingDir: ${CHE_PROJECTS_ROOT}/nodejs-web-app/app | ||
group: | ||
kind: build | ||
- exec: | ||
id: run the app | ||
commandLine: "nodemon app.js" | ||
component: runtime | ||
workingDir: ${CHE_PROJECTS_ROOT}/nodejs-web-app/app | ||
group: | ||
kind: run | ||
- exec: | ||
id: firstPostStartCmd | ||
commandLine: echo I am the first PostStart | ||
component: tools | ||
workingDir: ${CHE_PROJECTS_ROOT}/ | ||
- exec: | ||
id: secondPostStartCmd | ||
commandLine: echo I am the second PostStart | ||
component: tools | ||
workingDir: ${CHE_PROJECTS_ROOT}/ | ||
- exec: | ||
id: disconnectDatabase | ||
commandLine: echo disconnecting from the database | ||
component: tools | ||
workingDir: ${CHE_PROJECTS_ROOT}/ | ||
events: | ||
postStart: | ||
- "firstPostStartCmd" | ||
- "secondPostStartCmd" | ||
preStop: | ||
- "disconnectDatabase" | ||
``` | ||
|
||
The example flow for **odo push** in this case would be: | ||
- create the containers | ||
- execute firstPostStartCmd within the tools container | ||
- execute secondPostStartCmd within the tools container | ||
- execute build command | ||
- execute run command | ||
|
||
The example flow for **odo delete**, in this case, would be: | ||
- execute disconnectDatabase | ||
- clean up the pod and deployment etc. (as done today) | ||
|
||
## Future Evolution | ||
|
||
The initial implementation of postStart events assumes all containers are initilised during component startup (first push). In future we could make this granular and allow executing these postStart commands if containers are restarted by other commands/events too. | ||
|
||
Example: A potential flow could be to check for postStart commands associated with a specific container when initializing it, storing those commands in an Object(?) and then executing only those postStart commands when the containers have all finished initializing (in the order that they have been defined within the devfile). This would benefit cases where an individual container has been re-initialized, but not the whole component. For example, if a build container has been restarted as a part of **odo push --force** and there is a postStart command associated with the build container in the devfile that is required to re-run, that single postStart command would be executed before continuing to the build/run phase of the push command. | ||
|
||
Today, *preStop* and *postStop* events are supported in Devfile 2.0.0, but there is no clear use case for them in odo. As and when we have a clear use case, we should consider implementing those for odo too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should support this odo. This can be easily implemented as an initContainer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kadel Do you have a clear use case in mind for preStart, that can't be achieved by postStart?
We did consider init containers - convert devfile command into a command string and pass it as the command for the init container. But, weren't sure it would make sense if the command was going to use the same build/run container. So left it out as a future piece of work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, I don't.
But this is how it was defined in devfile. We should implement it. Supporting only half the events in odo is going to just confuse everyone who will try to create devfiles.
If there are no technical reasons not to implement it we should implement it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just not clear to me how we can handle cleanly in odo. If we try to support this and someone specify a command that uses the build/run container for the
preStart
, how do you even handle that? This command is meant to be called before those container get started. In order to execute those command, we need to start the container.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preStart
needs to be executed before the workspace pod (in odo case the component) is started. This is exactly whatinitContainer
does. Che is going to implement this in the same way. Check discussion in devfile/api#32There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried to re-read : devfile/api#32 and found something that I had missed earlier:
Implementation of preStart commands may need execution of a workspace pod init container. That means that the component associated to the preStart command should not be part of the workspace pod. The same applies to postStop bindings.
There is a clear requirement that container used for preStart and postStart commands will not overlap with containers of the main workspace pod. This makes is a bit clearer and perhaps we can implement preStart as init container or take an approach similar to postStop and run them as normal container commands before we run any postStart commands.
If there is no overlap in containers, there should not really be any difference in the outcome and it might be simpler to implement without init containers as we have all that we need for that already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elsony Please see my comment above. This is something that I had missed too, but the spec clearly states that there should not be any overlap of containers between main workspace containers (build, run, test etc.) and preStart, postStop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should go with preStart => initContainers and only run them when the component is bieng created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part might be tricky. I'm not sure if it will be possible. The initContainer will be executed everytime Deployment is triggered. Which might be right, or not :-( depending on what the command is doing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking that we could add init containers to the deployment.yaml only for the first push. and consecutive push, we can generate the deployment without init containers. But these can be looked at in more details during implementation phase as each will be implemented under separate issue.
For now we can update the proposal to include:
preStart
- execute commands via init containerspostStop
- execute commands via standalone pod or job and delete it once complete@elsony @kadel would this make sense?