-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
New path for distribution config #4365
New path for distribution config #4365
Conversation
The original path was referencing a docker directory which no longer makes much sense. Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
Won't this change break existing adopters who are mounting config into the image? Would a simple bash entrypoint to detect and use /etc/docker/registry be a good solution until 4.0.0? |
It will most certainly break yeah, but I figured if we pushed this out in beta, it could give people heads up before stable release. Hacking some shell commands (Seb mentioned them in the referenced issue) until v4 is out seemed like a bad idea to me given we're still gonna push beta out before the stable release and until stable is out we should try to make as many "breaking" changes as possible IMHO. That said I'm happy to be persuaded otherwise |
Personally, I feel there are levels of breaking change, and this would break anyone doing basic things like running in k8s and mounting a configmap. A simple script would allow a deprecation period we could write into the release notes, even if in the beta notes we say it will be removed in 3.0.0 final. We're in an odd situation, I expect we have more people running pre release than usual because of the long runway. |
Perhaps. Happy to put something together. I personally prefer getting a nudge (punch?) before stable 😃 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sense, LGTM
This is for v3.0, given it may have some break changes, it is still acceptable. Additionally, leaving the docker executable in the path is not reasonable. |
Yeah I've changed my mind on this too. I think we should go ahead with this in beta. Break beta, before stable is released. Otherwise we are carrying over some shell cruft which sure as heck won't be removed by god knows how long if ever 🙃 |
We have a couple breaking changes now in v3.0, if we document them I think it's fair, especially for a major release. |
@Jamstah I dont wanna merge this without looping you in and have your agreement as well since you raised valid points |
I still think we're in a weird situation where we should be more careful breaking the alpha/beta than normally. However, my opinion goes against that of the crowd, so I think you should merge it. |
The original path was referencing a docker directory which no longer makes much sense.
Closes #4357
Also see: distribution/distribution-library-image#166