-
Notifications
You must be signed in to change notification settings - Fork 170
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
Containerized portal as it runs in prod, locally #3745
Conversation
900ecfc
to
01f167f
Compare
/azp run ci |
Azure Pipelines successfully started running 1 pipeline(s). |
Makefile
Outdated
.PHONY: docker-secrets | ||
docker-secrets: aks.kubeconfig | ||
docker secret rm --ignore aks.kubeconfig | ||
docker secret create aks.kubeconfig ./aks.kubeconfig | ||
|
||
docker secret rm --ignore proxy-client.key | ||
docker secret create proxy-client.key ./secrets/proxy-client.key | ||
|
||
docker secret rm --ignore proxy-client.crt | ||
docker secret create proxy-client.crt ./secrets/proxy-client.crt | ||
|
||
docker secret rm --ignore proxy.crt | ||
docker secret create proxy.crt ./secrets/proxy.crt |
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.
IIRC we aren't supposed to use docker command any more.
https://www.redhat.com/sysadmin/podman-kubernetes-secrets
podman also has secret command. Doesn't it work?
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.
We could move this to strictly say podman
- it's been assumed for a while that all ARO devs are using podman-docker
library to effectively alias docker=podman
and not use the docker
binary - podman
is what I was testing with locally under the hood here.
I can update if we've decided to move away from the docker
alias - but docker
is ubiquitous throughout the Makefile today. If we want to do that, I think it should be done all at once and in a different PR maybe? I'd be +1 to do that because it's more clear that we're actually using podman
locally - but it may have downstream impacts on pipelines.
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.
FWIW +1 for being explicit with podman
and +1 for a separate PR to convert the lot
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.
If we concern downstream impacts, we can do one by one starting with this PR.
IMHO I don't like the way to do all at once because we can't identify what breaks what.
And also runlocal-portal
uses podman, so we're good to use podman here as well, I guess.
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.
I created a follow-up PR for this (and some other things I've noticed we need) #3775
01f167f
to
a26453e
Compare
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. I tested this out (manually by running the make targets' implementation directly against other portal PRs) and the portal worked fine for all our use cases.
Adding |
5c47b9a
to
789f41c
Compare
/azp run ci |
Azure Pipelines successfully started running 1 pipeline(s). |
Please rebase pull request. |
- creates a new make target to run the Portal app containerized - updates portal addresses to work with podman machine - adds docs
2640d2a
789f41c
to
2640d2a
Compare
/azp run ci |
Azure Pipelines successfully started running 1 pipeline(s). |
- creates a new make target to run the Portal app containerized - updates portal addresses to work with podman machine - adds docs
- creates a new make target to run the Portal app containerized - updates portal addresses to work with podman machine - adds docs
Which issue this PR addresses:
Fixes ARO-9500
What this PR does / why we need it:
You can now test portal changes locally the same way it's run in prod - via a container run process on
localhost:8444
Test plan for issue:
I need confirmation from MacOS users that this works for them - I updated some Portal server configs like we did for Shubhada's PR for RP (e.g. directly use
:8444
instead oflocalhost:8444
so it works with podman machine setups).I have validated on Linux that I can run:
make runlocal-portal
https://localhost:8444
to see the portal in action.Is there any documentation that needs to be updated for this PR?
Yes, it's in the PR.
How do you know this will function as expected in production?
This is still an unused container image - it is useful now in local development only and is only bringing conformity between prod and local dev.