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

Add option to use file with environment variables in new-app #7585

Closed
ppitonak opened this issue Feb 24, 2016 · 14 comments
Closed

Add option to use file with environment variables in new-app #7585

ppitonak opened this issue Feb 24, 2016 · 14 comments
Assignees
Labels
component/cli component/internal-tools help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/P3
Milestone

Comments

@ppitonak
Copy link

It is possible to specify environment variables directly to oc new-app command

$ oc new-app openshift/postgresql-92-centos7 \
-e POSTGRESQL_USER=user \
-e POSTGRESQL_DATABASE=db \
-e POSTGRESQL_PASSWORD=password

Add an option to specify a file with env variables so that they don't have to be specified on command line (was implemented in OpenShift 2, Docker has --env-file option)

$ oc new-app openshift/postgresql-92-centos7 --env-file /tmp/myenvironmen

where file would contain

POSTGRESQL_USER=user
POSTGRESQL_DATABASE=db
POSTGRESQL_PASSWORD=password
@mfojtik
Copy link
Contributor

mfojtik commented Feb 24, 2016

@ppitonak sounds reasonable to me. @smarterclayton || @fabianofranz ?

@fabianofranz
Copy link
Member

Yes. In rhc create-app you could either pass KEY=VALUE or the path to a file using the same --env flag, which is also completely feasible. No strong preference.

@smarterclayton
Copy link
Contributor

It's reasonable. Note that for new-app you don't even need -e. You can do

oc new-app POSTGRES_USER=hello ... openshift/postgresql-92-centos7

@fabianofranz fabianofranz added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label May 20, 2016
@bparees bparees self-assigned this Jul 7, 2016
@smarterclayton smarterclayton modified the milestones: 1.2.x, 1.3.0 Jul 12, 2016
@ravisantoshgudimetla
Copy link
Contributor

@bparees : I wish to contribute to this PR, I am new to Openshift and need you guys help. Let me know, if I can pick this up.

@bparees
Copy link
Contributor

bparees commented Aug 25, 2016

@ravisantoshgudimetla best place to start would be our contributor guide:
https://github.com/openshift/origin/blob/master/CONTRIBUTING.adoc

once you have a dev environment up and running we can discuss how to solve this issue.

@ravisantoshgudimetla
Copy link
Contributor

@bparees I have set the dev environment up. I am currently reading newapp.go in /go/src/github.com/openshift/origin/pkg/cmd/cli/cmd.

@bparees
Copy link
Contributor

bparees commented Aug 25, 2016

@ravisantoshgudimetla ok, that's the right place to start, i think the original issue summary describes the intent pretty well, so I guess just let me know where you have questions.

@ravisantoshgudimetla
Copy link
Contributor

@bparees Cool, I will go through it and get back to you, in case I have questions. Thanks.

@ravisantoshgudimetla
Copy link
Contributor

@bparees - Following are the set of changes that I thinking of making for this PR. The idea is to reuse the existing code as much as possible. I am trying to parse the environment file and get the []string from it and append it to environment []string in newapp.go .

cmd.Flags().StringSliceVarP(&config.EnvironmentFile, "envfile", “", config.Environment, “Specify location for key-value pair environment file”) in newapp.go in origin/pkg/cmd/cli/cmd
In origin/pkg/generate/app/cmd/newapp.go in AddArguments function following line is going to be added.
case app.IsPossibleEnvironmentFile(s):
c.Environment = append(c.Environment, app.getEnvironmentVarList(s)) //This is easiest but probably not consistent with whats already there.
In origin/pkg/generate/app/environment.go file is created, which has IsPossibleEnvironmentFile() which takes the file path and checks if its a file and getEnvironmentVarList which takes the file and reads the values in it line by line, have some validation to see if its of format key-value pair and return the []string which can be appended in c.Environment.

Please let know, if I am missing something or if there is an easier way to do this.

@bparees
Copy link
Contributor

bparees commented Aug 26, 2016

@ravisantoshgudimetla generally sounds reasonable, go ahead and open the PR and we'll continue the discussion from there.

@ravisantoshgudimetla
Copy link
Contributor

@bparees, as discussed raised PR. Please note that I haven't written the test cases yet. Once the approach seems reasonable , I will go ahead and write some test cases.

@jtslear
Copy link
Contributor

jtslear commented Dec 1, 2016

I know I'm horribly late to the game, but what if we simply improved the documentation, and relied on shell utilities to complete?
oc new-app $(cat test.env) --docker-image=jtslear/testapp

@bparees bparees assigned mmilata and unassigned bparees Dec 1, 2016
@bparees
Copy link
Contributor

bparees commented Dec 1, 2016

@mmilata the card you're working this sprint should resolve this.

@mmilata
Copy link
Contributor

mmilata commented Jan 17, 2017

Should be fixed in #12164.

@mmilata mmilata closed this as completed Jan 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/cli component/internal-tools help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/P3
Projects
None yet
Development

No branches or pull requests

10 participants