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

Docker publish does not write metadata to dist #19341

Open
bobthemighty opened this issue Jun 16, 2023 · 7 comments · May be fixed by #19346
Open

Docker publish does not write metadata to dist #19341

bobthemighty opened this issue Jun 16, 2023 · 7 comments · May be fixed by #19346
Labels
backend: Docker Docker backend-related issues bug

Comments

@bobthemighty
Copy link
Contributor

Describe the bug

pants package causes a docker target to write a metadata json including the tags and registries for the new image.

pants publish reports that it has created the metadata file in the sandbox, but the file is never written back to dist. This means I can't use it for fetching the tag I've just published and, since my tags contain timestamps, I can't package then publish.

Pants version
2.16.0rc5

@bobthemighty
Copy link
Contributor Author

Is there a historical deliberate reason to not write BuiltPackages to a Workspace as part of the Publish goal? If there's no strong reason for it, I think I an open a PR for this one pretty easily, by including the BuiltPackage in the PublishProcesses as part of package_for_publish, then writing it out from the goal_rule.... right?

@kaos
Copy link
Member

kaos commented Jun 16, 2023

Have you provided this option a value?
https://www.pantsbuild.org/docs/reference-publish#output

@bobthemighty
Copy link
Contributor Author

bobthemighty commented Jun 16, 2023

I looked at that, but that's a different behaviour. I can use it, from scripts, but it's not as clean as writing a separate manifest for each published image to a directory in dist.

My use case is that I'm deploying a bunch of serverless apps. If I have a well-known path for each image, it's easy to load the JSON manifest, extract the tagged image and use it in config.

That solution still works if somebody runs pants package src/some_dir, and happens automatically, so people don't need to remember to --output to a particular location.

If I use output and someone runs publish then unless they set the output correctly, and only try to deploy the one component they published, it'll go awry.

@kaos
Copy link
Member

kaos commented Jun 16, 2023

I see no reason right now not to write the manifest from the package step to dist also during publish.

since my tags contain timestamps, I can't package then publish.

I am curious though if this is not the behaviour you get with pants package publish ... i.e. run both goals with one invocation. Then publish should use the packaged images, and package would write the metadata to dist as expected.

If I use output and someone runs publish then unless they set the output correctly

This setting could go into pants.toml so they wouldn't have to worry about it.

@kaos
Copy link
Member

kaos commented Jun 16, 2023

Also, if you wish to package first, then later publish, you might just as well use the metadata from the package goal to just run docker push $image as that is basically just what pants does in that case any way.

(throwing around ideas to see what sticks)

@bobthemighty
Copy link
Contributor Author

I am curious though if this is not the behaviour you get with pants package publish ... i.e. run both goals with one invocation. Then publish should use the packaged images, and package would write the metadata to dist as expected.

Yes! Which is a neater way to handle my immediate problem, I guess. I can stick a wrapper script around it and move on. The context here is that I'd like the set up to work robustly in the face of people doing things manually and not just relying on The Blessed Script.

This setting could go into pants.toml so they wouldn't have to worry about it.

The problem with setting a static location for the publish output is that pants publish foo and pants publish bar will both write to that file, which means that cd foo && serverless deploy or cd bar && serverless deploy are then broken because they're missing data.

Also, if you wish to package first, then later publish, you might just as well use the metadata from the package goal to just run docker push $image as that is basically just what pants does in that case any way.

I could, but then I'm writing my own version of pants publish, and I guarantee you that some engineer will run pants publish anyway and be confused by what's happening.

IF you're open to writing metadata from publish, I think that gives me the most developer friendly way to tackle this issue.

I see no reason right now not to write the manifest from the package step to dist also during publish.

I'll try and put together a PR on that basis. This seems like it should be safe, and unsurprising, behaviour - if we package as part of publish, we'll write the same files to dist that we would have written for package alone.

@kaos
Copy link
Member

kaos commented Jun 16, 2023

I see no reason right now not to write the manifest from the package step to dist also during publish.

I'll try and put together a PR on that basis. This seems like it should be safe, and unsurprising, behaviour - if we package as part of publish, we'll write the same files to dist that we would have written for package alone.

Fair enough. Given all the work-arounds and this is still the preferred solution I'm +1, as I don't have all the circumstantial details here ;)

Thanks for tackling this.

@huonw huonw added the backend: Docker Docker backend-related issues label Jun 17, 2023
@bobthemighty bobthemighty linked a pull request Jun 17, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Docker Docker backend-related issues bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants