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.

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

odo config support for settings in devfile.yaml #2688

Closed
17 tasks done
elsony opened this issue Mar 6, 2020 · 25 comments · Fixed by #3702
Closed
17 tasks done

odo config support for settings in devfile.yaml #2688

elsony opened this issue Mar 6, 2020 · 25 comments · Fixed by #3702
Assignees
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. kind/user-story An issue of user-story kind priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)). v2 Issue or PR that applies to the v2 of odo

Comments

@elsony
Copy link

elsony commented Mar 6, 2020

User Story

As a developer, I want to configure my configuration settings of devfile style components in the devfile.yaml so that I can change the settings after the initial creation.

Background

The storing of the configuration settings for the devfile style components is currently stored in the devfile.yaml that follows the devfile specification. However, the user currently cannot edit the settings in using the command line after the devfile in the project has been created as part of other commands.

Acceptance Criteria

for "LocalConfig" components (s2i)

  • odo config should work as currently does

for devfile components

odo config

odo config is doing all operations on devfile.yaml, should not touch any other file.
Devfile can have multiple containers defined. odo config should by default apply the configuration to all containers in devfile.

  • odo config set Name - set metadata.name field i in devfile.yaml
  • odo config set --env FOO=bar - add FOO env variable with bar value to all containers defined in devfile
  • odo config set Ports 8080/TCP - add new endpoint to each container in devfile
  • odo config set Memory - set memoryLimit for all containers in devfile.yaml
  • odo config unset/view should work with new and updated fields
  • config options in odo confg should have new updated description to match its meaning in devfile spec
  • odo config set DebugPort - we can remove this option it in favor of odo config set --env DEBUG_PORT=12345
  • odo config set MaxCPU - remove this command - devfile v2 currently doesn't have cpu limits
  • odo config set MaxMemory - remove this command - devfile v2 currently have only memory limit
  • odo config set CPU - remove this command - devfile v2 currently doesn't have cpu limits
  • odo config set MinCPU - remove this command - devfile v2 currently doesn't have cpu limits
  • odo config set MinMemory - remove this command - devfile v2 currently have only memory limit
  • odo config set Ref - remove this command - not valid for devfile
  • odo config set SourceType - remove this command - not valid for devfile
  • odo config set Type - remove this command - not valid for devfile
  • odo config set SourceLocation - remove this command - not valid for devfile

The devfile reference

Links

/kind user-story
/area devfile

@openshift-ci-robot openshift-ci-robot added kind/user-story An issue of user-story kind area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. labels Mar 6, 2020
@cdrage
Copy link
Member

cdrage commented Mar 9, 2020

Sorry if I do not understand this, but at the moment, we already store environment variable information in the .odo/config.yaml file in the projects folder.

Are you proposing that we should have a separate file outside of .odo/config.yaml with the environment information?

Ex of our current implementation:

~/openshift/nodejs-ex  master ✗                                                                                                                                                                                                                                         42d ⚑ ◒  
▶ odo config set --env RAILS_ENV=development         
 ✓  Environment variables were successfully updated

Run `odo push --config` command to apply changes to the cluster

~/openshift/nodejs-ex  master ✗                                                                                                                                                                                                                                         42d ⚑ ◒  
▶ cat .odo/config.yaml 
kind: LocalConfig
apiversion: odo.openshift.io/v1alpha1
ComponentSettings:
  Type: nodejs
  SourceLocation: ./
  SourceType: local
  Ports:
  - 8080/TCP
  Application: app
  Project: testing
  Name: nodejs-nodejs-ex-oguc
  Envs:
  - Name: RAILS_ENV
    Value: development

@kadel
Copy link
Member

kadel commented Mar 11, 2020

Sorry if I do not understand this, but at the moment, we already store environment variable information in the .odo/config.yaml file in the projects folder.

Are you proposing that we should have a separate file outside of .odo/config.yaml with the environment information?

Yes, but only for devfile components. This is what Elson is proposing.

I've added 4 more acceptance criteria that should make this clearer.

But I'm not convinced that this is a good approach.

@elsony Do you think that it makes sense to store env variables in the environment-specific config file? Shouldn't be all env variables captured in devfile.yaml?

The way I see it odo config set --env key=value should modify the env variables in devfile.yaml. We will probably have to add something like --devfile-component flag fully define where the env variables should be.

@elsony
Copy link
Author

elsony commented Mar 11, 2020

I don't think we should store any true environment specific config in the devfile.yaml. It will make the devfile not portable and users can't check that into team repository. Having said that, there are some env setting that fits into the devfile, e.g. a devfile writer can have some containers rely on some env variable to change the behaviour of the build. Strictly speaking, that's part of the stack design and they are not really environment specific so those can be checked into the team repo and put as part of the devfile. However, for the real env specific that we currently put under the env.yaml, e.g. ingress domain, target namespace for odo push, etc., those are not portable to other system and we don't expect people to check it into the team repo. Therefore, the main difference between devfile.yaml and env.yaml are whether those config are environment neutral that can be reused in other systems.

Regarding to the concerns on odo config set --env key=value making changes on env.yaml vs devfile.yaml, I think the bigger question is whether we want odo to be the tool to edit the devfile. devfile is more than just for odo usage (also used by Che and Appsody, etc.) and it much more complex than the existing config.yaml. I think if we want a fully featured tools to help users out on editing the devfile, it should be a separate tool for doing that, e.g. a stack creator tool or something like that. odo will focus on things that we create/inject. Having said that, if we do find a few settings that a typically odo user will likely to edit, it will be fine if we allow user to do some selected odo config setting to edit things from the devfile for convenience.

@kadel
Copy link
Member

kadel commented Mar 13, 2020

I don't think we should store any true environment specific config in the devfile.yaml. It will make the devfile not portable and users can't check that into team repository. Having said that, there are some env setting that fits into the devfile, e.g. a devfile writer can have some containers rely on some env variable to change the behaviour of the build. Strictly speaking, that's part of the stack design and they are not really environment specific so those can be checked into the team repo and put as part of the devfile. However, for the real env specific that we currently put under the env.yaml, e.g. ingress domain, target namespace for odo push, etc., those are not portable to other system and we don't expect people to check it into the team repo. Therefore, the main difference between devfile.yaml and env.yaml are whether those config are environment neutral that can be reused in other systems.

Agreed, ingress domain, namespace are good examples of environment-specific configs. But environment variables are not environment-specific. Env variables are highly specific to a given application, they should be part of the application configuration, and as such, it should be captured in devfile.yaml, most of the application depend on the right env variables being set.

Regarding to the concerns on odo config set --env key=value making changes on env.yaml vs devfile.yaml, I think the bigger question is whether we want odo to be the tool to edit the devfile. devfile is more than just for odo usage (also used by Che and Appsody, etc.) and it much more complex than the existing config.yaml. I think if we want a fully featured tools to help users out on editing the devfile, it should be a separate tool for doing that, e.g. a stack creator tool or something like that. odo will focus on things that we create/inject. Having said that, if we do find a few settings that a typically odo user will likely to edit, it will be fine if we allow user to do some selected odo config setting to edit things from the devfile for convenience.

Agreed, devfile is much more complex and powerful than config.yaml, it doesn't make sense to provide odo commands to modify every devfile aspect, but I think that there should be commands to do most common operations like modifying env variables, cpu/memory limits ....

Originally odo was designed in a way that users shouldn't need to edit any yaml files. And allow users to slowly build their application in an interactive way. This made it appealing to a lot of people. config.yaml was meant to be just implementation detail, which is more or less hidden to normal uses. Devfiles are a different story, they should be exposed to users, but at the same time, we shouldn't force users to edit devfile.yaml for small incremental tasks, if they don't fee like it.

@cdrage
Copy link
Member

cdrage commented Mar 31, 2020

Hi @elsony I've assigned this to you. I think you've already implemented some of the user stories / criteria for this? Could you go through and see what could be checkmarked off?

EDIT: I will pick this up 👍

@girishramnani girishramnani assigned cdrage and unassigned cdrage Apr 1, 2020
@girishramnani girishramnani added the triage/needs-information Indicates an issue needs more information in order to work on it. label Apr 1, 2020
@cdrage
Copy link
Member

cdrage commented Apr 1, 2020

Alright, I've started work on this, here's my TODO list (for my own reference too):

When experimental mode is enabled, the following must happen against env.yaml not config.yaml:

  • odo config set --env key=value
  • odo config set --env key=value but override if key already exists
  • odo config unset --env key
  • odo config view

Other scenarios:

  • If .odo/env.yaml does not exist, tbut .odo/config.yaml is present, the previous above commands should work against LocalConfig
  • If there is no devfile.yaml or .odo/config.yaml show error about non-existing component
  • Obviously if experimental=false, do everything like normal.

Branch: https://github.com/cdrage/odo/tree/implement-env-setting-devfile

@girishramnani
Copy link
Contributor

so is any of the AC actually completed @cdrage?

@kadel
Copy link
Member

kadel commented Apr 2, 2020

I think that odo config will need much bigger overhaul :-(

Currently, odo supports the following parameters

  • Application - Application is the name of application the component needs to be part of
  • CPU - The minimum and maximum CPU a component can consume
  • DebugPort - The port on which the debugger will listen on the component
  • Ignore - Consider the .odoignore file for push and watch
  • MaxCPU - The maximum CPU a component can consume
  • MaxMemory - The maximum memory a component can consume
  • Memory - The minimum and maximum memory a component can consume
  • MinCPU - The minimum CPU a component can consume
  • MinMemory - The minimum memory a component is provided
  • Name - The name of the component
  • Ports - Ports to be opened in the component
  • Project - Project is the name of the project the component is part of
  • Ref - Git ref to use for creating component from git source
  • SourceLocation - The path indicates the location of binary file or git source
  • SourceType - Type of component source - git/binary/local
  • Storage - Storage of the component
  • Type - The type of component
  • URL - URL to access the component
  • Env - environment variables

With the old-style components (s2i) we have only one place to store configuration (config.yaml)

But with devfile components, we actually have two places where the config values can be stored env.yaml and devfile.yaml.
As I tried described in #2688 (comment) the configuration that is related to application configuration (for example resource limits or env variables as I see them) should be stored in devfile.yaml. devfile.yaml is supposed to describe how to set up the environment and how to run the source code and is meant to be shared.
For example, there might be env variable that needs to be set in order to application work properly, so this should be recorded in devfile.yaml, or if the application required at least of 1GB RAM, it should be specified in the devfile.yaml, otherwise, every user will have to set this in their env.yaml

I'm not so sure that just storing env variables set by odo config set --env key=value in env.yaml is a good solution.
And even if we go this route we need to clearly define how it will work, as devfile can define multiple containers, so does it mean that env variables from env.yaml will be set for all containers or just some?

@elsony
Copy link
Author

elsony commented Jun 22, 2020

As part of the devfile 2.0 spec (devfile/api#61), we are going to support .devfile.yaml in the project and handle it in the same way as the devfile.yaml. I think odo config should support both devfile.yaml and .devfile.yaml.

@gorkem
Copy link
Contributor

gorkem commented Jul 1, 2020

@kadel What is the expectation for odo debug port-forward command when determining the remote port? If odo config set DebugPort is retired in favor of setting the environment variable DEBUG_PORT. Does port-forward look only to the values on env.yaml ?

@kadel
Copy link
Member

kadel commented Jul 7, 2020

@kadel What is the expectation for odo debug port-forward command when determining the remote port? If odo config set DebugPort is retired in favor of setting the environment variable DEBUG_PORT. Does port-forward look only to the values on env.yaml ?

The remote port should be still determined by DEBUG_PORT environment variable.
Odo should check DEBUG_PORT env variable in the container and use its value as a remote port.

The idea is that the environment variable will make it easy to configure port in debug command.

schemaVersion: 2.0.0
metadata:
  name: nodejs
components:
  - container:
      name: nodejs
      image: nodejs
      mountSources: true
      env:
        -  name: DEBUG_PORT
           value: 1234
commands:
  - exec:
      id: debug
      component: runtime
      commandLine: nodemon --inspect 127.0.0.1:${DEBUG_PORT}
      group:
        kind: debug
        isDefault: true

@mik-dass
Copy link
Contributor

mik-dass commented Jul 7, 2020

The remote port should be still determined by DEBUG_PORT environment variable.
. Odo should check DEBUG_PORT env variable in the container and use its value as a remote port.

This needs to be well documented somewhere for users making the transition from s2i components.

@gorkem
Copy link
Contributor

gorkem commented Jul 7, 2020

OK, so debug port-forward is only meant to work with DEBUG_PORT. It appears that I was doing a hackish thing to forward different ports. See also #3495

@kadel
Copy link
Member

kadel commented Jul 20, 2020

@girishramnani This issue is still labeled as "needs-information". Is there something that is still needed?

@girishramnani girishramnani added triage/ready and removed triage/needs-information Indicates an issue needs more information in order to work on it. labels Aug 4, 2020
@girishramnani
Copy link
Contributor

I would have a PR for review in a day. Quite close to getting it done. 1 small thing that I have already done but just wanted to confirm - can someone have a devfile which isn't called devfile.yaml?

@girishramnani
Copy link
Contributor

Also --now flag for config might be a follow up PR

@rm3l rm3l added the v2 Issue or PR that applies to the v2 of odo label Jun 18, 2023
@rm3l rm3l added this to odo Project Jun 18, 2023
@rm3l rm3l moved this to Done ✅ in odo Project Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. kind/user-story An issue of user-story kind priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)). v2 Issue or PR that applies to the v2 of odo
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

9 participants