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

storewolf: declare dependencies on model and defaults files #1319

Merged
merged 1 commit into from
Feb 11, 2021

Conversation

tjkirch
Copy link
Contributor

@tjkirch tjkirch commented Feb 11, 2021

Description of changes:

storewolf: declare dependencies on model and defaults files

Without this, you can change a defaults file and not haven those changes in a
new build, or change the variant and still use the prior cached model link.

I believe this should also address the rare file-not-found issue seen in #1303 and in the checks of a couple PRs after.

Testing done:

Before, I built a k8s AMI and it was fine, but then I built an ECS AMI and it had the wrong model, spewing lots of errors about deserializing the incorrect data that storewolf had put in the data store. (@etungsten originally reported this behavior and I verified.)

After, I built a k8s AMI and it was fine, and then I built an ECS AMI and it was also fine, with the correct ECS settings.

Also confirmed that changes to the defaults files now rebuild:

  • cargo make build-packages
  • cargo make build-packages again; it did not rebuild os.
  • touch 10-defaults.toml, cargo make build-packages, it does rebuild os.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Without this, you can change a defaults file and not haven those changes in a
new build, or change the variant and still use the prior cached model link.
Copy link
Contributor

@webern webern left a comment

Choose a reason for hiding this comment

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

🎓

@bcressey
Copy link
Contributor

Can you confirm by checking the output of the os package build that the storewolf crate is not "fresh" and gets rebuilt after touch 10-defaults.toml?

@tjkirch
Copy link
Contributor Author

tjkirch commented Feb 11, 2021

Can you confirm by checking the output of the os package build that the storewolf crate is not "fresh" and gets rebuilt after touch 10-defaults.toml?

@bcressey I confirmed that a no-change rebuild gives me a "Fresh storewolf" and after touching 10-defaults.toml I get "Compiling storewolf".

@tjkirch tjkirch merged commit 31259db into bottlerocket-os:develop Feb 11, 2021
@tjkirch tjkirch deleted the storewolf-deps branch February 11, 2021 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants