-
Notifications
You must be signed in to change notification settings - Fork 58
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
proposal for env var sourcing [EE-2542] #94
base: master
Are you sure you want to change the base?
Conversation
In order to test it: I'm thinking we need to open another PR as an intermediary PR (based on this PR), e.g. target different files (suffixed by -dev or something), upload the files in the appropriate locations (download.portainer.io), update the Portainer code to reference this -dev suffixed script, test it. After validation, merge this PR and remove the intermediary PR changes. Thoughts @ssbkang @samdulam @yi-portainer? |
We only need to change the file location in the script to the manifest file in this branch. Once tested change it back before merge? |
TODO:
|
FYI, running the script gave me the following output @ssbkang :
|
Note that this is not required anymore for https://portainer.atlassian.net/browse/EE-2436 as we're planning to support Docker and Swarm to start with. We should keep it open as a reference for https://portainer.atlassian.net/browse/EE-2542 though. |
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.
LGTM
There is no need to merge this for the EE-2.12 release. |
I am also not sure yet why we have a whole bunch of files it looks good, just need to create files for the correct CE/EE version |
726a656
to
2c984d4
Compare
This PR proposes a way to load environment variables passed to the script with the new feature available through https://portainer.atlassian.net/browse/EE-2436
This feature introduce a new parameter for the script: a single string containing multiple environment variables separated by a comma (e.g.
VAR1,VAR2
).This is a proposal and feels a bit hacky, suggestions are welcome. Maybe we could generate a YAML file with the environment content and use
--from-file
instead of--from-literal
?I wasn't sure which file needed to be updated so I updated both.