-
Notifications
You must be signed in to change notification settings - Fork 23
feat: use RELATED_IMAGES_ environment variables to configure operand's images #812
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
Conversation
3057129 to
dc7b201
Compare
99976e7 to
1c8328a
Compare
1c8328a to
1a0f6ff
Compare
7e26eee to
83f4538
Compare
83f4538 to
fe9cc6c
Compare
|
/test tas-operator-e2e |
|
Openshift CI job will fail untill https://github.com/openshift/release/blob/master/ci-operator/config/securesign/secure-sign-operator/securesign-secure-sign-operator-main.yaml#L62-L83 will be modified to work with new changes. |
|
/override ci/prow/tas-operator-e2e |
|
@osmman: Overrode contexts on behalf of osmman: ci/prow/tas-operator-e2e In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
fe9cc6c to
595a830
Compare
| } | ||
|
|
||
| type registry struct { | ||
| mutex sync.RWMutex |
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.
why do we need synchronization? do you expect images to be changed on runtime?
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 added synchronisation because the values are configured via callbacks (async).
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.
Unfortunatly I forget the reason why I have to do that way. It has been a long time ago when I develop that.
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 reworked that registry to remove async setter but I think that it is bettor to keep synchronisation because registry is singleton. The async setter is looking to be left over from prototyping and hacking flag package.
| @@ -0,0 +1,20 @@ | |||
| apiVersion: v1 | |||
| data: | |||
| RELATED_IMAGE_BACKFILL_REDIS: registry.redhat.io/rhtas/rekor-backfill-redis-rhel9@sha256:6aa3ca40e0f9e32a0a211a930b21ff009b83e46609bfa5bb328979e4799d13c7 | |||
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 know that this is needed by Kustomize but I don't think it is necessary to keep it as part of the bundle. Maybe we can add it to .gitignore WDYT?
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 think that it will be better to keep it. It will be better to try keep kustomize output as close to bundle/. For example make build-installer will generate resources with that config.
5aaf2f9 to
978e195
Compare
|
/override ci/prow/tas-operator-e2e |
|
@osmman: Overrode contexts on behalf of osmman: ci/prow/tas-operator-e2e In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/override Red Hat Konflux / rhtas-operator-e2e-test / rhtas-operator |
|
/override "Red Hat Konflux / rhtas-operator-e2e-test / rhtas-operator" |
|
@osmman: Overrode contexts on behalf of osmman: Red Hat Konflux / rhtas-operator-e2e-test / rhtas-operator In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bouskaJ, osmman The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/override ci/prow/tas-operator-e2e |
|
@osmman: Overrode contexts on behalf of osmman: ci/prow/tas-operator-e2e In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
This pull request introduces a new feature that allows the Operator manager to utilize
RELATED_IMAGES_*environment variables for image configurations. This change allows operator-sdk to generate bundle image with relatedImages meta data which are used for disconnected environment. Additionally, it includes several refactoring changes to improve deployment for different environments Kubernetes/Openshift.Key Features
RELATED_IMAGES_environment variables for image configurations.CONFIG_DEFAULTfor dynamic configuration based on theOPENSHIFTvariable.internal/images.