k8s icon indicating copy to clipboard operation
k8s copied to clipboard

proposal for env var sourcing [EE-2542]

Open deviantony opened this issue 3 years ago • 7 comments
trafficstars

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.

deviantony avatar Feb 03 '22 22:02 deviantony

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?

deviantony avatar Feb 13 '22 18:02 deviantony

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?

samdulam avatar Feb 14 '22 00:02 samdulam

TODO:

  • [x] Revert curl https://raw.githubusercontent.com/portainer/k8s/fc0044800a8cb4be6c19f88aac1c696cef7ba916/deploy/manifests/agent/portainer-ce211-agent-edge-k8s.yaml to curl -L https://portainer.github.io/k8s/deploy/manifests/agent/portainer-ce211-agent-edge-k8s.yaml in this PR

  • [x] Revert curl https://raw.githubusercontent.com/portainer/k8s/fc0044800a8cb4be6c19f88aac1c696cef7ba916/deploy/manifests/agent/portainer-ce211-edge-agent-setup.sh to https://downloads.portainer.io/portainer-ce${agentVersion}-edge-agent-setup.sh in https://github.com/portainer/portainer/pull/6517 and https://github.com/portainer/portainer-ee/pull/1190

deviantony avatar Feb 14 '22 02:02 deviantony

FYI, running the script gave me the following output @ssbkang :

curl https://raw.githubusercontent.com/portainer/k8s/fc0044800a8cb4be6c19f88aac1c696cef7ba916/deploy/manifests/agent/portainer-ce211-edge-agent-setup.sh | bash -s -- 1ddb8ea3-ab2a-4668-a42a-078d2a82664e aHR0cHM6Ly8yMDYuMTg5LjkyLjIzMjo5NDQzfDIwNi4xODkuOTIuMjMyOjgwMDB8MTg6Zjg6Nzk6NjE6OTU6MTI6MGI6NzA6YjM6MTU6YjQ6ZGI6NjI6MDc6Njk6Zjl8Mw 1 HOST,MACHINE_ID
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  3544  100  3544    0     0   9844      0 --:--:-- --:--:-- --:--:--  9871
Downloading agent manifest...
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  2457  100  2457    0     0   7875      0 --:--:-- --:--:-- --:--:--  7875
Creating Portainer namespace...
namespace/portainer created
Creating agent configuration...
main: line 78: configmap/portainer-agent-edge: No such file or directory
Creating agent secret...
secret/portainer-agent-edge-key created
Deploying agent...
Warning: resource namespaces/portainer is missing the kubectl.kubernetes.io/last-applied-configuration annotation which is required by kubectl apply. kubectl apply should only be used on resources created declaratively by either kubectl create --save-config or kubectl apply. The missing annotation will be patched automatically.
namespace/portainer configured
serviceaccount/portainer-sa-clusteradmin created
clusterrolebinding.rbac.authorization.k8s.io/portainer-crb-clusteradmin created
service/portainer-agent created
deployment.apps/portainer-agent created

deviantony avatar Feb 14 '22 02:02 deviantony

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.

deviantony avatar Feb 16 '22 17:02 deviantony

There is no need to merge this for the EE-2.12 release.

deviantony avatar Feb 18 '22 04:02 deviantony

I wasn't sure which file needed to be updated so I updated both.

I am also not sure yet why we have a whole bunch of files ce11... instead of using git tags for that.

it looks good, just need to create files for the correct CE/EE version

chiptus avatar Mar 02 '22 06:03 chiptus