Skip to content
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

OSSM-110 add relatedImages to csv file #405

Merged
merged 2 commits into from
Apr 22, 2020

Conversation

rcernich
Copy link
Contributor

Signed-off-by: rcernich rcernich@redhat.com

@brianredbeard
Copy link
Contributor

Out of curiosity, why is viper being pulled in? I don't see any context in the commit message. Is this to support some later CLI mechanism for manual testing/execution?

@rcernich
Copy link
Contributor Author

rcernich commented Apr 17, 2020

I'm using viper to parse configuration for the operator. This allows specification of the image names in one of two ways: configuration file and environment variable. For this PR, I've elected to use a properties file to specify the related images into the deployment using a downward api volume. This made the deployment resource a lot less verbose (we have 23 related images).

For reference, here are some examples:

olm.relatedImage.v1_1.cni=registry.redhat.io/openshift-service-mesh/istio-cni-rhel8:1.1.0
export OLM_RELATEDIMAGE_V1__1_CNI="registry.redhat.io/openshift-service-mesh/istio-cni-rhel8:1.1.0"

HTH

@rcernich
Copy link
Contributor Author

FWIW, I had initially thought about adding the ability to pass the image names in as command line arguments, which is what initially got me looking at viper. In the end, it really came down to the fact that it supported reading in config through either an environment variable or file, and supported unmarshaling the parsed configuration into an object.

@rcernich rcernich changed the title WIP: OSSM-110 add relatedImages to csv file OSSM-110 add relatedImages to csv file Apr 20, 2020
@rcernich rcernich requested a review from luksa April 21, 2020 16:38
@rcernich rcernich assigned dgn and unassigned dgn Apr 21, 2020
@rcernich rcernich requested a review from dgn April 21, 2020 16:39
@luksa
Copy link
Contributor

luksa commented Apr 22, 2020

When creating a v1.0 SMCP, the first Deployment that is created has an invalid image (e.g. docker.io/maistra/registry.redhat.io/openshift-service-mesh/citadel-rhel8:1.0.10:1.0.10). The 1.0 charts don't check if the image name already contains the hub and tag. The 1.1 charts do.

Is there a PR that fixes this? I couldn't find it.

@luksa
Copy link
Contributor

luksa commented Apr 22, 2020

I've opened #411 to fix the 1.0 charts.

@rcernich
Copy link
Contributor Author

rcernich commented Apr 22, 2020

It should be part of this PR

ad13a12#diff-b8a523fbc9e23335e648496932633b06

@luksa
Copy link
Contributor

luksa commented Apr 22, 2020

Oh, weird. I ran the operator from this PR locally and it created the istio-citadel deployment with an invalid image name. Looks like I forgot to regenerate the charts in tmp/_output/. Seems to work properly now.

Signed-off-by: rcernich <rcernich@redhat.com>
Signed-off-by: rcernich <rcernich@redhat.com>
@rcernich rcernich removed the request for review from dgn April 22, 2020 18:11
@mergify mergify bot merged commit a96d1b7 into maistra:maistra-1.1 Apr 22, 2020
@rcernich rcernich deleted the OSSM-110 branch April 22, 2020 18:12
rcernich added a commit to rcernich/istio-operator that referenced this pull request Apr 22, 2020
* OSSM-110 add relatedImages to csv file

Signed-off-by: rcernich <rcernich@redhat.com>

* add github.com/spf13/viper module

Signed-off-by: rcernich <rcernich@redhat.com>
mergify bot pushed a commit that referenced this pull request Apr 29, 2020
* OSSM-110 add relatedImages to csv file

Signed-off-by: rcernich <rcernich@redhat.com>

* add github.com/spf13/viper module

Signed-off-by: rcernich <rcernich@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants