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

Strip build- prefix from docker image task labels #585

Merged
merged 2 commits into from
Oct 25, 2024

Conversation

ahal
Copy link
Collaborator

@ahal ahal commented Oct 7, 2024

No description provided.

@ahal ahal added the BREAKING CHANGE Backwards incompatible request that will require major version bump label Oct 7, 2024
@ahal ahal self-assigned this Oct 7, 2024
@ahal
Copy link
Collaborator Author

ahal commented Oct 7, 2024

There's no rush on this, just wanted to tee it up for the next time we're doing a breaking change anyway.

@jcristau
Copy link
Contributor

jcristau commented Oct 8, 2024

I guess this is to align with gecko?

@ahal
Copy link
Collaborator Author

ahal commented Oct 9, 2024

Yeah, it's blocking us consolidating the docker image logic as this prefix is hardcoded in a few places.

I guess we could make it a configuration option, but I don't like that Taskgraph defaults to build- as a prefix anyway, as this makes it easy to confuse them as the build kind if someone is parsing labels for the kind (which they shouldn't do, but happens in practice).

Copy link
Contributor

@jcristau jcristau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

ahal added 2 commits October 25, 2024 14:57
BREAKING CHANGE: `build-` prefix stripped from docker image tasks

This is being done to facilitate merging some docker related logic with
gecko_taskgraph. The issue with adding a `build-` prefix to the labels,
is that it is common for there to exist a `build` kind.

While it isn't best practice to parse labels to obtain information like
the kind, in reality this happens pretty frequently. So making this
change:

1. Makes the label consistent with Gecko (needed to merge some docker
   related logic)
2. Makes the label consistent with other tasks (prefix should just be
   the kind name)
3. Reduces footguns for folks that are parsing the label.
Opening file paths here results in an OSError in Gecko due to the large
number of files involved in creating docker images there.

The underlying tar writer accepts both file paths or file objects, so
switch to paths to avoid the error in Gecko.
@ahal ahal force-pushed the docker_image_name branch from 9a3f17b to 9234c49 Compare October 25, 2024 18:58
@ahal ahal merged commit a39ae14 into taskcluster:main Oct 25, 2024
16 of 17 checks passed
@ahal ahal deleted the docker_image_name branch October 25, 2024 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE Backwards incompatible request that will require major version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants