Add dockerfile for Rust container + setup

[MarijnS95]

Now that we're progressing towards build-testing Rust bindings in the Khronos CI, a minimal docker image with Rust support and few other dependencies is needed to make this possible.

[CLAassistant]

All committers have signed the CLA.

[MarijnS95]

@oddhack if I understand the build failure correctly, it wants Docekrfile.openxr-base but only Dockerfile.openxr-base.202110 is provided.

[rpavlik]

you are right, failure to maintain the pipeline scripts since they were not doing what we wanted to begin with. https://github.com/KhronosGroup/DockerContainers/pull/14 should fix that.

[rpavlik]

ok rebase or merge from master, and you should have an OK time with CI. You may want to add your new images to CI however.

[MarijnS95]

@rpavlik Thanks for addressing this so quickly, that's about what I expected it would take for a solution, though I would have been blunt and removed the date instead.

I'll see what it takes to "add the image to CI", I had hoped this would be included through the script.

[MarijnS95]

Added it, but I don't see what's up with the extra step of pulling the base image (docker pull ruby:2.3, which I would replicate as docker pull rust:latest); shouldn't the underlying docker build do that automatically?

[rpavlik]

yeah it should do that, I don't remember why I do that manually.

Also, I don't remember why I'm not just using the scripts. It would be good to just port this to use github actions and just run the scripts.

The reason the dates are there, is because it's hard to get CI runners to update their image version otherwise. If they have cached an old version, we need something in the image name to bump in order to trigger them to fetch images anew instead of using the cached version.

[MarijnS95]

That may be a good idea, yes. I also haven't understood the difference between Dockerfile.rust and rust.dockerfile - I followed the latter to match asciidoctor-spec (since this is used in similar CI scripts) but Azure seems to want the former.

(Seems like it's at least used to determine whether to come up with a default tag).

For this to make sense though we should probably unify the build and build-and-push scripts to take an argument whether to push or not, so that we don't have to replicate build-and-push-all.sh with all its lines as that may change at some point.

---

Regarding versions, I've seen this in other CI's as well; seems good practice to assign a specific version tag in addition to latest, but we should also make sure to not replicate these all across the codebase. At the same time, shouldn't caching layers at least consult with dockerhub whether their view of the latest tag hasn't changed? Seems common practice to have this, maybe a --pull argument is missing somewhere.

[rpavlik]

I'm not aware of any functional difference between the two naming conventions you mention.

Yeah, version + latest tag would be good, just haven't gotten around to it since these images are primarily consumed by CI rather than humans.

If you can figure out how to get the kubernetes gitlab runner to always check the registry for updated images before running, please let us know :) James and I couldn't find any way to do so, which is why I fell back to the date/version tag thing.

[MarijnS95]

@rpavlik Just as you posted this I gobbled together an initial version that unifies the scripts, naming, and version tagging - including also adding a -latest tag: https://github.com/KhronosGroup/DockerContainers/pull/15

I don't have a clue for GitLab runner though, other than that it feels like this should be handled internally as the default... Perhaps there's a way to specify a request for "pull" or tag sync or however that's called :)

[rpavlik]

very cool, thanks for the PR! I do indeed like it, nicer than the current mostly-bodge.

[oddhack]

@rpavlik Just as you posted this I gobbled together an initial version that unifies the scripts, naming, and version tagging - including also adding a -latest tag: #15

I don't have a clue for GitLab runner though, other than that it feels like this should be handled internally as the default... Perhaps there's a way to specify a request for "pull" or tag sync or however that's called :)

@rpavlik the gitlab runner configuration has a parameter runners.docker.pull_policy which can be set to always; was that an option when you last talked with @outofcontrol about this? Might be the right answer, although hopefully it also respects image caching so it isn't pulling hundreds of MB from dockerhub on every CI run. IIRC at one point we dealt with the out-of-date image issue by just restarting the runners but that's a big hammer.

I think there's finer control in github Actions CI at the CI YML configuration stage but haven't fully sorted that out yet.

[MarijnS95]

It looks like always is the default anyways - and that should only cause it to check for tag updates but not redownload if it's in the cache, otherwise a cache is pretty much meaningless :)

[rpavlik]

I don't recall if that was an option, I think maybe it wasn't, or it was already set to "always". The difference between "updating asciidoctor" and what we're bumping the version/date for, is that we'll have gitlab changes that require a corresponding change to the image. Even on other GitLab instances, where we can maintain the container in the same repo as the rest of the code by pushing to gitlab registry, we still have to use "decorated" tag names to ensure that a branch is built with a suitable image. (The reason I hadn't split the image name and tag name before is that in theory, we could keep the old tag names around, updating their contents without changing the list of packages, but I'm not aware of any non-CI usage of the images other than on my desktop, so that hasn't been a priority.)

[MarijnS95]

@oddhack @rpavlik Looks like the CI also succeeds here :)

[rpavlik]

Looks good, only one suggestion but I'm fine if you want to merge this as-is.

[MarijnS95]

Ah, you're hitting the same GitHub snafu! ctrl+enter closes a PR now, whereas it's only supposed to do that with ctrl+shift+enter.

[rpavlik]

Well that's annoying. Reopened for ya

[MarijnS95]

Thanks! With two approvals we should be good to merge :grimacing:

[MarijnS95]

@rpavlik Many thanks for merging! Who should I ask to publish this image to dockerhub, you or @oddhack?

[oddhack]

I'm publishing it now... ... done, looks OK on khronosgroup/docker-images:rust.202206

[MarijnS95]

@oddhack Thanks! Looking at what has been published thus far, specifically the asciidoctor-spec tag, I should've probably left out the .latest suffix on all images in #15. That only makes sense if the name of the image is included in the repo and the tag is purely for versioning, like KhronosGroup/rust:latest. Afaik a tag is mandatory here.

@rpavlik mentioned that it may have been a quota thing, can any of you check that? If that's the case I'll open a PR to delete the .latest tag, otherwise I'll open a PR to switch to a KhronosGroup/rust:latest format for every image. Then we may also decide to delete the arguably messy tags before people start to rely on them?

[rpavlik]

Looks like we no longer have a limit on repos, just on pulls per day and simultaneous builds (and users).

[oddhack]

@oddhack Thanks! Looking at what has been published thus far, specifically the asciidoctor-spec tag, I should've probably left out the .latest suffix on all images in #15. That only makes sense if the name of the image is included in the repo and the tag is purely for versioning, like KhronosGroup/rust:latest. Afaik a tag is mandatory here.

I found the script as it stands wasn't doing what I wanted with asciidoctor-spec, since I wanted it updated without the suffix, so I pushed it manually. I think having options to the build-one.sh to set the desired suffix(es) would be useful.

Do you want just 'rust.202206' or just 'rust' or what desired end state for this image in the repo?

Do you have a dockerhub account? Happy to set you up with a khronosgroup/rust repository you can push to - it would just take slightly more scripting work. If so then email me your dockerhub handle and I'll do that.

[MarijnS95]

@oddhack Thanks! Looking at what has been published thus far, specifically the asciidoctor-spec tag, I should've probably left out the .latest suffix on all images in #15. That only makes sense if the name of the image is included in the repo and the tag is purely for versioning, like KhronosGroup/rust:latest. Afaik a tag is mandatory here.

I found the script as it stands wasn't doing what I wanted with asciidoctor-spec, since I wanted it updated without the suffix, so I pushed it manually. I think having options to the build-one.sh to set the desired suffix(es) would be useful.

Same here, I don't really see why I added .latest instead of leaving it out altogether. Those feelings apply to all images, not just asciidoctor-spec, so I'd rather remove it again while we still can instead of adding extra options.

Do you want just 'rust.202206' or just 'rust' or what desired end state for this image in the repo?

I think it'd be great to have both, just not rust.latest anymore.

Do you have a dockerhub account? Happy to set you up with a khronosgroup/rust repository you can push to - it would just take slightly more scripting work. If so then email me your dockerhub handle and I'll do that.

Can we do that for all repositories (and leave KhronosGroup/docker-images for legacy usage) so that we can update the scripts at once without fallbacks for rust specifically?

[MarijnS95]

I've made PRs for both options: #16 and #17.

[oddhack]

Can we do that for all repositories (and leave KhronosGroup/docker-images for legacy usage) so that we can update the scripts at once without fallbacks for rust specifically?

I'm not certain what you're asking - do you want write access to all (all 3 :-)) khronosgroup dockerhub repositories? Or if you're suggesting that every API get its own repo for images, or that every image get its own repo, I don't see the compelling use case. Maybe our naming scheme isn't aligned with dockerhub standard practice, but it hasn't caused any issues yet, either. The script just needs to know to what repo each image belongs, also easily passed on the command line (personally, I've largely switched to writing Python scripts for this sort of infrastructure just because argument parsing and simple data structures and logic are so much easier to read/write than bash, but whatever works).

[MarijnS95]

I'm not certain what you're asking - do you want write access to all (all 3 :-)) khronosgroup dockerhub repositories?

Someone else can do it as well. Would mainly like to delete the noisy .latest tags again if we were to merge #16 too.

Or if you're suggesting that every API get its own repo for images, or that every image get its own repo, I don't see the compelling use case.

That is exactly what I'm suggesting. If you don't agree, let's drop #17 and go ahead with #16?

EDIT: My initial intent was to create a single repo per image, but it seems we can also create a repo per API at the detriment of script/naming complexity.

The script just needs to know to what repo each image belongs, also easily passed on the command line

I don't want to have different repository naming standards for the files in this github repo. Either everything stays on KhronosGroup/docker-images:<name>[.<version>], or we move everything to KhronosGroup/<name>:{latest,<version>}.

(personally, I've largely switched to writing Python scripts for this sort of infrastructure just because argument parsing and simple data structures and logic are so much easier to read/write than bash, but whatever works).

Fine by me, feel free to convert them :)

[rpavlik]

so typical convention is at least to have only one dockerfile per directory to avoid polluting the "context". the "repo part" of the docker image name isn't necessarily corresponding to a single git repo, but usually it does correspond to a single logical image with potentially different versions.

I have absolutely no preference for how to set it up, though. As I mentioned, as long as there's something I can change in the name:tag combo to trigger a fetch on the runners, it's good enough for me and the openxr use case.

[MarijnS95]

the "repo part" of the docker image name isn't necessarily corresponding to a single git repo, but usually it does correspond to a single logical image with potentially different versions.

Yeah, that's a clearer way to describe it.