Repository navigation
Extend deployByDefault to other components, drop the implicit apply command rule #852
Description
Activity
I would love to work on this issue. @l0rd
Hey @l0rd , Ohh, I am bit late to know about this project. Hey, Myself vivek !! Currently a student, looking for new project to contribute. And it seems to me an interesting project. I have bit of experience of past one year in open source world. Contributed to Kyverno project. Looking forward to know more about this project. Any useful resources ??
Hi @l0rd , this is Anurag Bhat , a student looking to contribute to this project under the LFX mentorship program . I feel this project is a great entry point for me into cloud native development, after contributing to couple of GSoC orgs in the past year. I had submitted my application 3-4 days ago and believe the admission process is in progress now !
Thanks .I'm not a big fan of the "apply rule" so it would be great to have something more explicit.
I'm a bit afraid of creating too many rules like "If this is enabled then you can't enable something else" it might make writing devfiles too complex as it createss too many invalid scenarios. I would avoid adding validation like this unless it creates situation that are ambiguous or completely invalid.
- a container with
runOnDemand: falseis referenced by an apply a command
Doing this might look weird, but essentially it is not creating an invalid scenario. You will be applying something that already exists, so it will only result in "noop".
- a kubernetes/openshift with
deployByDefault: trueis referenced by an apply command
Similar in as above scenario
- a image with
autobuild: trueis referenced by an apply command
I can actually see a valid scenario where I would want to build image with
autobuild: trueagain. I would setautobuild: trueto have it build for the first time on startup, and later I might want to apply that image component again to re-build image, when I did some changes.name
runOnDemandlooks a bit weird to me, It might not be obvious what it does.Could we maybe name this field
autoStartand have the logic reversed.
By default it would befalseso it would be applied only if it is referenced by command.
autoStart: truewould mean that is is automatically applied on workspace start, but it could be still referenced by commands.I will try to create a few examples to verify how it would work.
- a container with
@kadel setting
autoStart: falseby default would add some syntax sugar AND change the semantic of existing Devfile that do not have anautoStartfield. Although this is an interesting topic (are devfile containers started more often at startup or after running a command?) I would like to keep this issue about a new more explicit syntax without hanging the behavior of existing Devfiles.I am ok to call it
autoStartand it's a good idea to allow a component to haveautoStart: trueand to be referenced by anapplycommand for now. We can decide later, based on feedback, if we need to avoid it or not.@rajibmitra is going to work on this issue as part of the LXF mentorship program.
Reacted by rajib@kadel setting
autoStart: falseby default would add some syntax sugar AND change the semantic of existing Devfile that do not have anautoStartfield. Although this is an interesting topic (are devfile containers started more often at startup or after running a command?) I would like to keep this issue about a new more explicit syntax without hanging the behavior of existing Devfiles.I got a little bit carried away 😇 . You are right this might not be a good idea right now as it would be incompatible with current Devfile behaviors, but it would simplify a lot of things :-)
I am ok to call it
autoStartand it's a good idea to allow a component to haveautoStart: trueand to be referenced by anapplycommand for now. We can decide later, based on feedback, if we need to avoid it or not.I'm afraid that if we want to keep default behaviors as
autoStart: truewe will need a different name for this field :-/
Some time back we agreed that we should try to keep default values forbooleanfieldsfalse, as it makes it much easier to deal with them.
This means that we won't be able to useautoStart. We will need a different name that will have inverted logic (like the originally proposedrunOnDemand).
runOnDemandsounds a little bit weird to me, but currently, I can't think of a better name for this.During the Devfile Cabal of the 20th of June we have discussed a couple of proposal as mentioned in another issue to use the field
deployStrategyinstead ofrunOnDemand:Proposal 1:
DeployStrategy: AtStartupInMainPod, AtStartupInSeparatePod, StartWithCommandInMainPod, StartWithCommandInSeparatePodProposal 2:
DeployStrategy: AtStartup, StartWithCommand
DedicatedPod: true/false@kadel Action: Come up with a proposal and examples on how these fields will behave.
# Controlling component creation
Devfile has 4 types of components:
containerimagekubernetesopenshiftvolume
Current situation
By default, there is an "apply" rule: If a component is not referenced by an
applycommand, the component will be automatically created on startup.On top of that there is also
deployByDefaultfield forkubernetesandopenshiftcomponents andautoBuildforimagecomponent.If
deployByDefaultorautoBuildistrue, then the component will be created on startup regardless if it is referenced by theapplycommand or not.kubernetescomponentdeployByDefault: truedeployByDefault: falsedeployByDefaultnot setreferenced in apply command automatically created on startup created only when the command is invoked created only when the command is invoked not referenced in apply command automatically created on startup never started ??? automatically created on startup imagecomponentautoBuild: trueautoBuild: falseautoBuildnot setreferenced in apply command automatically created on startup created only when the command is invoked created only when the command is invoked not referenced in apply command automatically created on startup never started ??? automatically created on startup Originally Proposed change
Introduce new field
deployStrategywith optionsatStartup,onDemanddeployStrategy: atStartupdeployStrategy: onDemanddeployStrategynot setreferenced in apply command automatically created on startup created only when the command is invoked created only when the command is invoked not referenced in apply command automatically created on startup never created automatically created on startup For backward compatibility can't remove
deployByDefaultandautoBuildfields.
They should be marked as deprecated in the docs. But we will have to keep supporting them.Setting both
deployByDefault/autoBuildanddeployStrategyshould be considered invalid. The validator or tools should error out if Devfile uses the old and new fields at the same time in the same component.containercomponentsFor the
containercomponent, theexeccommand should create theonDemandcontainer the same wayapplydoes.Problems
There is one big problem with
containercomponent andonDemandbehavior.It is not possible to add containers to the existing Pod without restarting Pod.
Let's say that
execcreatesonDemandcontroller the same wayapplywould.example
components: - name: runtime container: image: registry.access.redhat.com/ubi8/nodejs-16:latest endpoints: - name: http-3000 targetPort: 3000 - name: test container: image: registry.access.redhat.com/ubi8/nodejs-16:test deployStrategy: onDemand endpoints: - name: http-3000 targetPort: 3000 commands: - id: install exec: component: runtime commandLine: npm install workingDir: ${PROJECT_SOURCE} group: kind: build isDefault: true - id: run exec: component: runtime commandLine: npm start workingDir: ${PROJECT_SOURCE} group: kind: run isDefault: true - id: test exec: component: test commandLine: npm run test workingDir: ${PROJECT_SOURCE} group: kind: test isDefault: true
deployStrategycould be problematic forcontainerasonDemandwill properly work only if the main Pod gets restarted.
The solution could be that we define thatonDemandautomatically meansdedicatedPod: true, or we makeonDemandonly possible ifdedicatedPod: true, but at that point, we are creating a lot of rules that are not directly expressed in the file.kubernetes/openshiftcomponentsCreate
kubernetes/openshiftmeans simply "apply" the inlined or uri referenced definition.
There is no problem in doing it at the startup or on demand later on.imagecomponentsCreate
image/ means build the container image and push it to the registry.
There is no problem in doing it at the startup or on demand later on.
Devfile author just needs to make sure that ifcontainerorkubernetescomponent uses that image the build is executed first.volumecomponentsCreating volume on demand might be problematic. In most cases,
volumewill be represented asPersistentVolumeClaimresource on the cluster.
CreatingPVCwithout using it in the container doesn't make much sense.
There is no benefit in controlling when thevolumewill be created. Better would be to simply define that thevolumewill be created when thecontainerthat uses it first is created.deployStrategydoesn't make sense forvolumeProposal v2
Each component, where makes sense, will get its own field to control "deployStrategy".
Mario's
runOnDemandmakes sense forcontainercomponent.containercomponentsoption 1:
runOnDemand: true/false- When
runOnDemand: true, the container is always created in a separate pod. - By default, containers are started in one pod at startup.
execcommand should behave likeapplycommand forcontainercomponent (container "onDemand" will be just beforeexeccommand is executed
runOnDemand: truerunOnDemand: falserunOnDemandnot setreferenced in apply command created when apply command is invoked automatically created on startup created when apply command is invoked not referenced in apply command never started ??? automatically created on startup automatically created on startup dedicatedPod:true&runOnDemand:true: OK
dedicatedPod:true&runOnDemand:false: OK
dedicatedPod:true&dedicatedPod:false&runOnDemand:true: NOTVALID
dedicatedPod:false&runOnDemand:false: OK
dedicatedPod:false&kubernetes/openshiftcomponentsdeployByDefault: true/falsedeployByDefault: truedeployByDefault: falsedeployByDefaultnot setreferenced in apply command automatically created on startup created when apply command is invoked created when apply command is invoked not referenced in apply command automatically created on startup never started ??? automatically created on startup imagecomponentsautoBuild: true/falseautoBuild: trueautoBuild: falseautoBuildnot setreferenced in apply command automatically created on startup created when apply command is invoked created when apply command is invoked not referenced in apply command automatically created on startup never started ??? automatically created on startup volumecomponentsno field to control the creation; it gets created automatically when it is used in the container definition.
Just copying the final proposal to the comment to make it easier to link to.
To see how I got to this point check previous comment #852 (comment)
Proposal v2
Each component, where makes sense, will get its own field to control "deployStrategy".
Mario's
runOnDemandmakes sense forcontainercomponent.container:runOnDemandkuberetes/openshift:deployByDefualtimage:autoBuildvolume: nothing
containercomponentsoption 1:
runOnDemand: true/false- When
runOnDemand: true, the container is always created in a separate pod. - By default, containers are started in one pod at startup.
execcommand should behave likeapplycommand forcontainercomponent (container "onDemand" will be just beforeexeccommand is executed
runOnDemand: truerunOnDemand: falserunOnDemandnot setreferenced in apply command created when apply command is invoked automatically created on startup created when apply command is invoked not referenced in apply command never started ??? automatically created on startup automatically created on startup dedicatedPod:true&runOnDemand:true: OK
dedicatedPod:true&runOnDemand:false: OK
dedicatedPod:true&dedicatedPod:false&runOnDemand:true: NOTVALID
dedicatedPod:false&runOnDemand:false: OK
dedicatedPod:false&kubernetes/openshiftcomponentsdeployByDefault: true/falsedeployByDefault: truedeployByDefault: falsedeployByDefaultnot setreferenced in apply command automatically created on startup created when apply command is invoked created when apply command is invoked not referenced in apply command automatically created on startup never started ??? automatically created on startup imagecomponentsautoBuild: true/falseautoBuild: trueautoBuild: falseautoBuildnot setreferenced in apply command automatically created on startup created when apply command is invoked created when apply command is invoked not referenced in apply command automatically created on startup never started ??? automatically created on startup volumecomponentsno field to control the creation; it gets created automatically when it is used in the container definition.
Reacted by Stephanie CaoCommunity call discussion:
We will move this out of 2.2.- added a commit that references this issue
on Mar 21, 2023 - added a commit that references this issue
on Apr 5, 2023 This issue is stale because it has been open for 90 days with no activity. Remove stale label or comment or this will be closed in 60 days.
Which area this feature is related to?
/area api
/area library
/area devworkspace
Which functionality do you think we should add?
Why is this needed? Is your feature request related to a problem?
Devfile adopters struggle to understand what components will be started when a workspace is provisioned.
Detailed description:
Understanding which components are started automatically (or builded in the case of an image) and which are not is currently complicated.
There is an implicit rule to start only components that are NOT referenced by a command of type
apply. That's a hidden rule that is hard to understand and remember.The new
deployByDefaultandautoBuildfields have improved the situation but only apply to kubernetes/openshift and image components respectively.Describe the solution you'd like
Add a
runOnDemandboolean field for components of type containers (falseby default).Do not rely on the
applyrule anymore, but forbid the following configurations (document in the spec, raise an error in the library):runOnDemand: falseis referenced by an apply a commanddeployByDefault: trueis referenced by an apply commandautobuild: trueis referenced by an apply commandDescribe alternatives you've considered
We should add a new command of type
deletethat would be the complement ofapply.Related issues
#204
#693