Update Docker registry to new default #413
Conversation
bradfordcp
force-pushed
the
registry-changes
branch
2 times, most recently
from
9d21c8b to
4ba9eb9
Compare
Makefile
Outdated
|
|
||
| ORG ?= k8ssandra | ||
| ORG ?= us-docker.pkg.dev/k8ssandra/images |
There was a problem hiding this comment.
This will break all local builds.
There was a problem hiding this comment.
Why? It is working for me locally. The presence of a domain only affects pulling and pushing. I can import this package into Kind without issue.
There was a problem hiding this comment.
It's only working for certain use-cases. See, now you're overriding the default repository - not just the org. If I want to use a localhost:5000 repository (local registry) - which I've setup for example, this won't work. The images now become: localhost:5000/us-docker.pkg.dev/k8ssandra/images - and this is invalid.
Same applies to the image_config.yaml. If you wish to override the default repository, use the imageRegistry field, otherwise everyone who uses their own local registry (not just localhost, but say a proxy also) will not be able to use that feature anymore. Our parsing does not understand that there's repository applied in the wrong field - it assumes the format for certain image structures is organization/image and the additional repository comes from a different property.
There was a problem hiding this comment.
Alright, I'm understanding the relationships in image_config.yaml but there's a bunch of overlap. Why are we baking in the ORG as well, shouldn't that be part of the registry definition?
So if I want to use docker.io why not just specify imageRegistry: docker.io/k8ssandra with the image name system-logger:latest?
There's another gotcha in that we don't host the DSE images within the k8ssandra org, there isn't a way for me to specify the imageRegistry for particular images and not others. Should we mirror the the images with management API to our org?
There was a problem hiding this comment.
For the Makefile, should we add a REGISTRY parameter that can be overridden?
| @@ -3,7 +3,7 @@ kind: ImageConfig | |||
| metadata: | |||
| name: image-config | |||
| images: | |||
| system-logger: "k8ssandra/system-logger:latest" | |||
| system-logger: "artifacts.k8ssandra.io/k8ssandra/images/system-logger:latest" | |||
There was a problem hiding this comment.
Same here, won't work in local builds correctly.
There was a problem hiding this comment.
Also, if you wonder why to my previous question - the repository is different here than anything I would build. In the previous Makefile we build:
us-docker.pkg.dev/k8ssandra/images/system-logger:latest (which itself is problematic, but see another comment), but here we're calling artifacts.k8ssandra.io as prefix. So while your development env will popup, it's not actually loading the correct image, it's loading some historic one from the official repo, which is not what I'm actually developing.
There was a problem hiding this comment.
To be clear, us-docker.pkg.dev and artifacts.k8ssandra.io are the same repository. We want to push all images to us-docker.pkg.dev and pull from artifacts.k8ssandra.io.
There was a problem hiding this comment.
But that's not the same string - thus if you push there and load from another, you will not load from say local cache (like kind).
bradfordcp
force-pushed
the
registry-changes
branch
from
4ba9eb9 to
e58a2f4
Compare
bradfordcp
changed the title
bradfordcp
force-pushed
the
registry-changes
branch
2 times, most recently
from
43ef52c to
7601e11
Compare
Tests and defaults refer to artifacts.k8ssandra.io/k8ssandra/images Makefile updates for new repository locations
bradfordcp
force-pushed
the
registry-changes
branch
from
7601e11 to
3e2fe36
Compare
|
I'm still confused with why we want to change images.go and other processing, or set the default image names to be different. Why won't adding them to GHA and just setting a new imageRegistry to image_config.yaml be enough? Isn't the point that users just download from artifacts.k8ssandra.io? All the other changes seem unnecessary. |
At this point the only change in images.go is to set
Does this statement include the changes to push packages to |
I mean, only GHA needs to push there (that's the GHA part). But since Makefile's pushes are used only for local development work, there's no need to push with longer names there. One thing I obviously forgot is that the pre-release.sh needs the change to modify the cass-operator's deployment image to use the artifacts.k8ssandra.io/ .. path for the cass-operator image. The DSE image is more interesting - I didn't think about that. Are we intending to keep them on different repos? |
What this PR does:
Updates the registry coordinates to refer to the new Google Artifact Registry
GHA writes images to us-docker.pkg.dev/k8ssandra/images
Tests and defaults refer to us-docker.pkg.dev/k8ssandra/images
Makefile updates for new repository locations
Fixes #414
Checklist