-
Notifications
You must be signed in to change notification settings - Fork 73
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
oc-mirror configurability #775
Conversation
support configuration via environment variables instead of config file. this way deployment via azure container app should be more convenient Signed-off-by: Gerd Oberlechner <[email protected]>
image-sync/oc-mirror/main.go
Outdated
@@ -0,0 +1,62 @@ | |||
package main |
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.
Nit: I not have used golang for this, but plain .sh
, since it's available as well. 🤷♂️
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.
What about an alternative approach:
- Use tempaltize to generate the configuration
- Encode it base64, pass it in as environment variable
- Decode and write it to a file
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 like the base64 option. i'll go for it
Signed-off-by: Gerd Oberlechner <[email protected]>
--set azureTenantId=$${TENANT_ID} \ | ||
--set ocmirrorImage="arohcpdev.azurecr.io/image-sync/ocmirror" \ | ||
--set ocmirrorTag=latest \ | ||
--set credsPullSecret=pull-secret \ | ||
--set credsKeyVaultName=service-kv-aro-hcp-dev | ||
--set credsKeyVaultName=aro-hcp-dev-svc-kv |
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.
should this be a param?
@@ -1,9 +1,9 @@ | |||
FROM mcr.microsoft.com/cbl-mariner/base/core:2.0 AS downloader | |||
FROM --platform=linux/amd64 mcr.microsoft.com/cbl-mariner/base/core:2.0 AS downloader |
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.
Do we need to specify platform here? Should the container infer it depending on the platform you run on?
@@ -9,7 +9,6 @@ OC_MIRROR_IMAGE_TAGGED ?= $(OC_MIRROR_IMAGE):$(COMMIT) | |||
build-push: image push | |||
|
|||
image: | |||
cp ../configuration/mvp-oc-mirror.yml config.yml | |||
docker build --platform="linux/amd64" -f "./Dockerfile" -t ${OC_MIRROR_IMAGE_TAGGED} . |
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.
Docs say "podman" can we update docker
-> podman
or vice versa? Or do we have a preference?
platform: | ||
architectures: | ||
- multi | ||
- amd64 |
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.
Do we need amd64 images still, or can we switch everything to multi yet?
Please rebase pull request. |
closing this as it has been implemented as discussed in the #762 |
What this PR does
support configuration via environment variables instead of config file. this way deployment via azure container app should be more convenient
Jira:
Link to demo recording:
Special notes for your reviewer