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

build: support local state group #2038

Merged
merged 6 commits into from
Sep 30, 2023
Merged

Conversation

crazy-max
Copy link
Member

follow-up #1735

We currently support saving local state for a build ref but we can't associate multiple refs when building a group of targets. This PR adds support for saving the local state group so we can identify refs that belong to the same group of targets.

@crazy-max crazy-max added this to the v0.12.0 milestone Sep 11, 2023
build/build.go Outdated Show resolved Hide resolved
localstate/localstate.go Show resolved Hide resolved
localstate/localstate.go Outdated Show resolved Hide resolved
@crazy-max crazy-max force-pushed the localstate-group branch 3 times, most recently from a384ee3 to d1921b6 Compare September 12, 2023 18:09
@crazy-max
Copy link
Member Author

crazy-max commented Sep 12, 2023

Reworked a bit build invocation in first commit as BuildWithResultHandler had a bunch of args already so we can now pass variadic opts when instantiating Build instead. Wanted this as I was thinking passing a localstateHandleFunc but would need #1966 first as resolving the node of the build before saving its state would be necessary but seems enough to just pass localstate.StateGroup object instead.

Now it saves the raw definition, inputs and targets for the group. Also needs to save the target for each state ref so we can associate them with the definition.

Didn't set yet hcl variables but I guess we need a new Variables field to bake.Config. It would be ok for HCL definitions but would not have any meaning for compose files.

commands/bake.go Outdated Show resolved Hide resolved
@crazy-max crazy-max marked this pull request as ready for review September 25, 2023 13:24
@crazy-max
Copy link
Member Author

@tonistiigi As discussed, remove variadic opts and adds another arg to build.Build. Also needs to set build ref(s) early as well which requires a new Ref field to build options.

@crazy-max crazy-max force-pushed the localstate-group branch 5 times, most recently from 231ed85 to b335f04 Compare September 26, 2023 08:01
bake/bake.go Outdated Show resolved Hide resolved
build/build.go Outdated Show resolved Hide resolved
bake/bake.go Outdated Show resolved Hide resolved
commands/bake.go Outdated Show resolved Hide resolved
commands/build.go Outdated Show resolved Hide resolved
@crazy-max crazy-max force-pushed the localstate-group branch 2 times, most recently from 1e345f6 to ebd0a72 Compare September 29, 2023 08:08
build/build.go Outdated Show resolved Hide resolved
localstate/localstate.go Outdated Show resolved Hide resolved
Definition []byte
// Targets are the targets invoked
Targets []string `json:",omitempty"`
// Inputs are the user inputs (bake overrides)
Copy link
Member

Choose a reason for hiding this comment

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

So no variables? Ok by me to leave this for follow-up but comments seem to suggest that this was already added.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will look in follow-up. It's a bit tricky as the matrix feature hacks around variables so we can't just pick them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think if you want the values that the variables are evaluated to, you'd need to modify hclparser.Parse to return that information. I don't think there's a way to extract the vars/funcs defined in the HCL today 😢

Apologies, the code around there is a total mess - happy to help out 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

you'd need to modify hclparser.Parse to return that information.

Yes indeed, I have started to look at it and how I could use context evaluation to extract vanilla variables. Will open a PR and let you know, thx!

@crazy-max crazy-max merged commit 6c77b76 into docker:master Sep 30, 2023
59 checks passed
@crazy-max crazy-max deleted the localstate-group branch September 30, 2023 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants