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

Use a separate files for Kubernetes resources #4886

Closed
4 tasks
kadel opened this issue Jul 7, 2021 · 12 comments · Fixed by #5031
Closed
4 tasks

Use a separate files for Kubernetes resources #4886

kadel opened this issue Jul 7, 2021 · 12 comments · Fixed by #5031
Assignees
Labels
estimated-size/M (10-20) Rough sizing for Epics. About 1 sprint of work for one person estimated-size/S (5-10) Rough sizing for Epics. Less then one sprint of work for one person kind/user-story An issue of user-story kind

Comments

@kadel
Copy link
Member

kadel commented Jul 7, 2021

/kind user-story

User Story

As an odo user, I want to have Kubernetes resources in a separate file to easily review them or use 3rd party tools to deploy them.

Acceptance Criteria

  • odo link should put ServiceBinding into a separate file and use uri to reference it in kubernetes devfile component.
  • odo service create should put service CR into a separate file and use uri to reference it in kubernetes devfile component.
  • HTTPS URIs should be supported
  • Default should be URI with an option to let user specify --inlined

implementation notes:

  • generated yaml files should be placed in a separate directory to avoid polluting project root. For example ./kubernetes/ by default. This directory should be configurable using global odo preference.
  • filenames should use a pattern that is descriptive and avoids conflicts
  • commands like odo service list or odo describe needs to be able to handle a situation when kubernetes component is referenced using uri field.

example

odo service create  postgresql-operator.v0.1.1/Database -p databaseName=mydb -p databaseUser=user -p databasePassword=pass

Instead of inlining Operator CR into a devfile it will create kubernetes directory (if it doesn't already exist) and save Operator CR to kubernetes/database.yaml
Than it will add it as a kubernetes component into a devfile.yaml

....
- kubernetes:
    uri: kubernetes/database.yaml
...

/kind user-story

@openshift-ci openshift-ci bot added the kind/user-story An issue of user-story kind label Jul 7, 2021
@kadel kadel added this to the 2.4 (planning) milestone Jul 7, 2021
@scottkurz
Copy link
Contributor

scottkurz commented Jul 8, 2021

@kadel - would you agree a consequence of refactoring into a separate yaml file, referenced by 'uri' would be that devfile variable substitution can no longer be performed ?

Not sure yet what percentage of the time you'd want this, but it suggests maybe there should be an option to inline the included component in the original file?

Or could there be some other mechanism to "parameterize" the database.yaml in this example with a variable value?


And maybe I'm thinking about this wrong... if it's odo generating the yaml how would it know to parameterize it with a devfile variable? I probably just answered my own question but will leave the comment in case it's interesting.

@kadel
Copy link
Member Author

kadel commented Jul 9, 2021

@kadel - would you agree a consequence of refactoring into a separate yaml file, referenced by 'uri' would be that devfile variable substitution can no longer be performed ?

I need to verify that. I don't think that variable substitution works in inlined Kubernetes resources.
Ok, it works.

And maybe I'm thinking about this wrong... if it's odo generating the yaml how would it know to parameterize it with a devfile variable? I probably just answered my own question but will leave the comment in case it's interesting.

odo is not generating variables in devfile, and even with the current approach if you want to use variables in inlined Kubernetes manifest you would have to go and edit it manually.

I don't expect that using variables in kubernetes resources is something that odo users would do. This will be more common for people writing stacks.

If there is a need for using inlined manifest, we can introduce --inlined flag for odo link and odo service create

@dharmit
Copy link
Member

dharmit commented Jul 22, 2021

URI: https://docs.devfile.io/devfile/2.1.0/user-guide/api-reference.html

Default should be URI with an option to let user specify inlined.

@dharmit dharmit self-assigned this Jul 26, 2021
@dharmit
Copy link
Member

dharmit commented Jul 29, 2021

/triage needs-information

@openshift-ci openshift-ci bot added the triage/needs-information Indicates an issue needs more information in order to work on it. label Jul 29, 2021
@dharmit dharmit removed the triage/needs-information Indicates an issue needs more information in order to work on it. label Jul 29, 2021
@dharmit dharmit removed their assignment Jul 29, 2021
@dharmit dharmit added milestone/2.4 estimated-size/M (10-20) Rough sizing for Epics. About 1 sprint of work for one person labels Aug 3, 2021
@dharmit
Copy link
Member

dharmit commented Aug 12, 2021

Few notes:

  • We want to store service/link info in URI by default and not in devfile.yaml as Kubernetes Inlined component. However, if a stack author has provided it as inlined component, odo should still be able to create service upon odo push.
  • URI also means that we support https for the case where a devfile stack author has provided a URL

/triage needs-information

Set triage to ready if there's nothing missing in terms of acceptance criteria.

@openshift-ci openshift-ci bot added the triage/needs-information Indicates an issue needs more information in order to work on it. label Aug 12, 2021
@dharmit
Copy link
Member

dharmit commented Aug 16, 2021

generated yaml files should be placed in a separate directory to avoid polluting project root. For example ./kubernetes/ by default. This directory should be configurable using global odo preference.

Why not use the existing .odo directory that we already use to store env.yaml and odo-file-index.json? I'm also wondering if there's a pressing use case for making it configurable at global preference level. If not, I would like to keep it simple and not make it configurable till a use case pops up. I'm proposing this simply in favour of "Keep It Simple Silly".

What about tracking the file(s) via VCS? Putting it under .odo would mean not tracking it, IIUC. If we don't track it, the same git repo can't be used by multiple users to recreate things with a single odo push. Then it makes sense to put it under a different directory than .odo (.kubernetes as suggested in the issue description). However, in the documentation, we need to make sure to tell the user to not add this location to their .gitignore.

filenames should use a pattern that is descriptive and avoids conflicts

Would <component-name>-<operator-name>-<cr-name>.yaml suffice? Or should we try to have the same approach as we do for naming the kubernetes cluster resources like pvc, services, etc.?

commands like odo service list or odo describe needs to be able to handle a situation when kubernetes component is referenced using uri field.

What about odo service delete? Should it only delete the service from cluster and its URI reference from devfile.yaml, or should it also delete the file corresponding to the URI? IMO, it shouldn't delete the file. But then what if the user re-executes odo service create for the same <component-name>-<operator-name>-<cr-name>.yaml combination? Should odo silently override the existing file, or should it prompt the user? If odo must prompt the user, do we want to add --force flag to odo service create? Does it make sense to do so, or should it always prompt in such cases?

What about --from-file flag? IMO, if user tries to do below:

odo service create --from-file /path/to/some-file.yaml

odo should copy it over to .kubernetes/some-file.yaml and not try to apply <component-name>-<operator-name>-<cr-name>.yaml convention to it.

OTOH, if a user tries to do below, what would odo do?

odo service create --from-file .kubernetes/some-file.yaml
  • should odo throw an error because the user's trying to use the path .kubernetes with --from-file?
  • or, should odo just update the devfile.yaml and not try to copy the file since it's already under .kubernetes directory?

@kadel
Copy link
Member Author

kadel commented Aug 16, 2021

Why not use the existing .odo directory that we already use to store env.yaml and odo-file-index.json? I'm also wondering if there's a pressing use case for making it configurable at global preference level. If not, I would like to keep it simple and not make it configurable till a use case pops up. I'm proposing this simply in favour of "Keep It Simple Silly".

The files are there so other tools to use it, or for example for users to modify it.
Putting it into .odo would make it odo specific. We don't want that.

Also, the directory should not by default. The whole point of this is to make the files visible for users. kubernetes could be a good starting point.

What about tracking the file(s) via VCS? Putting it under .odo would mean not tracking it, IIUC. If we don't track it, the same git repo can't be used by multiple users to recreate things with a single odo push. Then it makes sense to put it under a different directory than .odo (.kubernetes as suggested in the issue description). However, in the documentation, we need to make sure to tell the user to not add this location to their .gitignore.

It will be up to the user whatever he/she wants to include it in .gitignore.

Would <component-name>-<operator-name>-<cr-name>.yaml suffice? Or should we try to have the same approach as we do for naming the kubernetes cluster resources like pvc, services, etc.?

This looks ok.

What about odo service delete? Should it only delete the service from cluster and its URI reference from devfile.yaml, or should it also delete the file corresponding to the URI? IMO, it shouldn't delete the file. But then what if the user re-executes odo service create for the same <component-name>-<operator-name>-<cr-name>.yaml combination? Should odo silently override the existing file, or should it prompt the user? If odo must prompt the user, do we want to add --force flag to odo service create? Does it make sense to do so, or should it always prompt in such cases?

odo service delete should ask user if it should delete the Kubernetes files or not, similarly how it is done in

▶  odo delete -a
? Are you sure you want to delete the devfile component: java-springboot-spring-petcl-umbb? Yes

Gathering information for component java-springboot-spring-petcl-umbb
 ✓  Checking status for component [14ms]

Deleting component java-springboot-spring-petcl-umbb
 ✓  Deleting Kubernetes resources for component [5ms]
 ✓  Successfully deleted component

Deleting local config
? Are you sure you want to delete env folder? No
 ✗  Aborting deletion of env folder
? Are you sure you want to delete devfile.yaml? No
 ✗  Aborting deletion of devfile.yaml file
▶ odo service delete MariaDB/mydb
? Are you sure you want to delete MariaDB/mydb Yes
Service "MariaDB/mydb" has been successfully deleted; do 'odo push' to delete service from the cluster

? Are you sure you want to delete 'kubernetes/petclinic-mariadb-mydb.yaml' file? Yes
Service "MariaDB/mydb" has been successfully deleted; do 'odo push' to delete service from the cluster

odo service delete -f MariaDB/mydb will delete everything without asking.

What about --from-file flag? IMO, if user tries to do below:

odo service create --from-file /path/to/some-file.yaml

odo should copy it over to .kubernetes/some-file.yaml and not try to apply <component-name>-<operator-name>-<cr-name>.yaml convention to it.

odo can even try to convert it to the same name pattern as other files. It has all the necessary information.

OTOH, if a user tries to do below, what would odo do?

odo service create --from-file .kubernetes/some-file.yaml
  • should odo throw an error because the user's trying to use the path .kubernetes with --from-file?
  • or, should odo just update the devfile.yaml and not try to copy the file since it's already under .kubernetes directory?

Throw an error

@feloy
Copy link
Contributor

feloy commented Aug 18, 2021

@dharmit Do we add these acceptance criteria?

  • Default must be URI with an option to let user specify --inlined
  • HTTPS URIs must be supported

@dharmit
Copy link
Member

dharmit commented Aug 18, 2021

Default must be URI with an option to let user specify --inlined

I don't know, tbh. Do we need really this flag in odo commands? The way I understand things is if there's a devfile that has inlined k8s components, odo will create those just the way it does now. But as far as odo link & odo service create are concerned, they should store them in .kubernetes/<file-name>.yaml.

I haven't looked deep into the variable thing mentioned in #4886 (comment). But does odo support configuring variables yet? odo should. But that's a separate issue, IMO.

Having --inlined flag would be, in that case, most beneficial to devfile stack authors. But they are anyway well-versed with things and could put it in the devfile themselves.

@kadel wdyt?

HTTPS URIs must be supported

Yes. 👍🏾

@kadel
Copy link
Member Author

kadel commented Aug 19, 2021

I don't know, tbh. Do we need really this flag in odo commands? The way I understand things is if there's a devfile that has inlined k8s components, odo will create those just the way it does now. But as far as odo link & odo service create are concerned, they should store them in .kubernetes/<file-name>.yaml.

I think that having an --inlined flag would be beneficial for users. There might be users that still prefer to have everything in one compact file.

Also, there is only a little extra work needed to implement this flag, we already have all the necessary functionality for this.

@feloy feloy added triage/ready and removed triage/needs-information Indicates an issue needs more information in order to work on it. labels Aug 19, 2021
@feloy feloy removed their assignment Aug 19, 2021
@dharmit dharmit added the estimated-size/S (5-10) Rough sizing for Epics. Less then one sprint of work for one person label Aug 19, 2021
@mik-dass
Copy link
Contributor

mik-dass commented Aug 23, 2021

The file schema of URI is not supported in the devfile api yet according to devfile/api#321 (comment)

@mik-dass
Copy link
Contributor

The file schema of URI is not supported in the devfile api yet according to devfile/api#321 (comment)

The scheme uses URI references and not absolute URIs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
estimated-size/M (10-20) Rough sizing for Epics. About 1 sprint of work for one person estimated-size/S (5-10) Rough sizing for Epics. Less then one sprint of work for one person kind/user-story An issue of user-story kind
Projects
None yet
5 participants