search

irods / irods_testing_environment

BSD 3-Clause "New" or "Revised" License
8 stars 15 forks source link

Add release Dockerfiles #117

Closed alanking closed 2 years ago

alanking commented 2 years ago

This approach scales better and provides a little more flexibility than the previous iteration (see #114).

We may wish to discuss default behaviors and how these options are specified.

I am also a little unsure about the use of environment variables in this way for the build-args, but it makes things a lot more flexible this way.

With all this in mind, leaving in draft again :)

korydraughn commented 2 years ago

I don't see a problem with using environment variables. My questions are:

alanking commented 2 years ago

Is the dockerfile environment variable safe from the user's environment (i.e. collisions)?

If the user has defined dockerfile or irods_package_version in their environment and uses the --use-static-image option, they will be ignored because the options set the environment variables before invoking up. This means that --use-static-image is basically a convenience for the user.

Having the environment variables set manually and using other options could lead to unexpected results. For instance, it may be possible to bypass using some options by setting the environment variables and this could allow for using incompatible parameters and triggering unexplored code paths.

I'm open to ideas about what this means and how much to protect the user from this, if at all.

What is the alternative to using environment variables?

We could use the Compose python API to define a dockerfile and the build-args. This makes me uneasy as the Compose python API usage in this project is unsupported and never has been. According to the Compose maintainers, the python API has never been supported as such and enforced this by dropping it entirely in V2 (see https://github.com/irods/irods_testing_environment/issues/8#issuecomment-1021320731).

We could also define separate Compose projects for the release images on each platform, but that seemed even less palatable.

The environment variables were inspired by other Compose projects I've worked with (namely, ZMT).

I'm open to implementing these or any other ideas.

What default behavior are you referring to?

I was mainly thinking about what our expectations are around what happens when some options are used and others are not.

For instance, as we discussed "offline" this morning, does it make sense to require --irods-package-version with --use-static-image or should we provide a strong default for --irods-package-version so that it's not required?

Or, should --use-static-image be the default in some cases (e.g. run_plugin_tests.py) and we provide a different option to download and install packages at runtime (e.g. --use-dynamic-image)?

korydraughn commented 2 years ago

I don't think we have to worry about the environment variables. Adding a few statements about those would be good. If we tell the reader that dockerfile and irods_package_version should not be defined in their testing environment, then I think we're covered.

As for the alternative (i.e. compose api), I agree that attempting to use that api is not in our best interest given it has been dropped in V2.

As for defaults, requiring users to pass --use-static-image feels correct. Developers are less likely to run the wrong version of iRODS that way. If a user provides the static image option, the package version should default to the latest release for that platform IMO. The idea that a user should opt into a static image also fits the purpose of this repo too.

alanking commented 2 years ago

Okay, so, I think the next steps include the following...

alanking commented 2 years ago

I created #121 to address the bit about the default behavior of --irods-package-version. This is going to be more of a lift because even if you don't specify irods_package_version environment variable, a blank string will be used instead which is obviously not good.

korydraughn commented 2 years ago

This seems like it's in a good spot.

Anything left to do for this?

alanking commented 2 years ago

Everything else left to do has been captured in an issue, I believe. I think it's good to go, too