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-3669: initial changes for interop pipeline support #517

Merged
merged 3 commits into from Apr 25, 2023

Conversation

FilipB
Copy link
Contributor

@FilipB FilipB commented Apr 24, 2023

We need an image which can be run on OCP. This is first change which is required. It contains also some minor improvements.

@FilipB FilipB requested review from luksa and yxun April 24, 2023 12:17
@openshift-ci openshift-ci bot added the size/M label Apr 24, 2023
@FilipB
Copy link
Contributor Author

FilipB commented Apr 24, 2023

There will be probably more changes, we just need this one merged to be able to start with the interop openshift CI config draft.

Dockerfile Outdated
Comment on lines 37 to 38
# using CMD here so it can be easily overwritten when using this in OpenShiftCI
CMD ["/bin/bash", "-c", "scripts/runtests.sh"]
Copy link
Contributor

@luksa luksa Apr 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this change prevents you from running the image like this:

podman run <image> TestFaultInjection

You're now forced to do it like this:

podman run <image> scripts/runtests.sh TestFaultInjection

This requires knowledge of the image internals (the name and location of the runtests.sh script).

What exactly do we need to overwrite when using this in OpenShiftCI?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was thinking that using TEST_CASE env variable would be good enough, we can describe this in the Readme to use only env variables when working with the container. My understanding is that in openshiftCI config you specify which commands need to be run in the container. Those commands then overwrite the default CMD. I need to check if it's possible to make it work also when using ENTRYPOINT e.g. via --entrypoint

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if people end up using the image even when running tests manually (so that they don't have to install any prerequisites), it has to be simple to use (e.g. without the TEST_CASE variable)... but it is true, that they may still want to set other variables...

As for OpenShiftCI: that's exactly the reason why I used ENTRYPOINT instead of CMD - because you will never want to run anything else than runtests.sh, but you may want to set additional arguments via CMD.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's use ENTRYPOINT for now. I still need to understand OpenShift CI better. We can change it later if it's going to be really needed.

@FilipB FilipB requested a review from luksa April 25, 2023 07:10
@FilipB FilipB removed the request for review from yxun April 25, 2023 07:38
@openshift-merge-robot openshift-merge-robot merged commit 2c51277 into maistra:maistra-2.4 Apr 25, 2023
3 checks passed
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

3 participants