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

Golden Path Changes - Part 2 #481

Merged
merged 18 commits into from
Mar 15, 2024
Merged

Golden Path Changes - Part 2 #481

merged 18 commits into from
Mar 15, 2024

Conversation

crhntr
Copy link
Member

@crhntr crhntr commented Mar 6, 2024

TL;DR: This change set includes a new command, minor changes to kiln bake, and some public changes to the Go packages.

The theme for these changes is supporting the centralized build system for Tanzu Tiles.

Tiles will now be (sha256) identical between kiln versions.

Dev Notes

Omit Kiln Version from Product Template

kiln bake no longer adds the kiln version to the product template. To keep track of which kiln version was used to bake a tile, authors should use the --final flag and commit the resulting bake record to the repository. I considered falling back to add the kiln_version when the --final flag was not used but can add that later if needed and want to propose the smallest change set.

Add a kiln re-bake Command

The new command kiln re-bake takes a path to a bake record and builds a tile byte for byte like it was built before. I intend to merge this into the kiln bake command but the control flow in the bake command is not clear and this proposed new command keeps the change as small as possible. As bake is refactored, we can merge in the functionality with reduced risk.

Add Tile Path to Bake Record

The Bake record now keeps track of a relative path from the repository root to the tile root (aka where the Kilnfile exists). This will make restructuring tile repos easier and reduce coupling between Golden Path configuration and tile repositories across revisions.

Create a bake Package

This will be the destination for refactored baking code. This package currently contains the bake.Record which is a data-structure for checksums and metadata about a tile build. The bake record maps source revisions, kiln versions, and tile checksums.

@crhntr crhntr added the tas-slingshots Created by https://github.com/orgs/pivotal-cf/teams/tas-strategic-initiatives-slingshot label Mar 6, 2024
@cf-gitbot
Copy link
Member

We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.

The labels on this github issue will be updated when the story is started.

@crhntr crhntr force-pushed the slingshots-bake-changes-02 branch 6 times, most recently from cd63543 to a25fec2 Compare March 6, 2024 03:40
crhntr added 11 commits March 8, 2024 11:32
I am refactoring towards exposing more of the bake functionality as a package.
…n versions

this commit removes the kiln version from kiln_metadata field in the baked product templates

the bake records can now be used to determine the kiln version for a particular bake
this will allow tile authors to move things around and still get consistent builds
this command takes a bake record and rebuilds a tile
this is an additional (useful) commit to ensure the previous acceptance is not brittle to checksum changes
the config field should match that this is not adding dirs but files
we no longer have submodules and we should parse the Go version from the mod file
I am not sure what is causing this difference
@crhntr crhntr force-pushed the slingshots-bake-changes-02 branch from 54580e4 to 78ce891 Compare March 8, 2024 19:36
@crhntr crhntr force-pushed the slingshots-bake-changes-02 branch from 34a3744 to 8471642 Compare March 8, 2024 20:23
@pabloarodas
Copy link
Contributor

We reviewed the PR with Riz and Preethi and we think the changes are ok. Derek gave us with some context about the Golden path that helped us understand better the reason behind these changes, however, one feedback we have for future PRs, is to try and separate feature work from refactors since those make the PRs too big and not so easy to review.

@crhntr please let us know when you finish doing changes in the PR and mark it as Ready for Review so we can go ahead and merge it.

@crhntr crhntr force-pushed the slingshots-bake-changes-02 branch from 0fb9a83 to df1e96b Compare March 9, 2024 02:24
i think this might be an issue with the sha256 of the tile
I would think it would also change the source_revision but maybe not
@crhntr crhntr force-pushed the slingshots-bake-changes-02 branch from df1e96b to ae535f0 Compare March 9, 2024 02:27
this adds confusion not sure why it is still here. The tests pass without it.
@crhntr
Copy link
Member Author

crhntr commented Mar 11, 2024

I spent all day Friday trying to figure out why the acceptance test was sometimes failing on new commits in CI. I did not figure it out. I will try to pick it up on Wednesday.

@ram-pivot was super helpful in figuring this out. We got it now! The fix was to now just use unix epoc instead of commit timestamp. "zeroing" time stamps is how Ko does this when you don't override the timestamp. I suspect we don't need to add a flag to override the zero value. Pairing across time zones made finding this bug much easier."

zipinfo -v and diff were the main tools we used to figure out the problem.

crhntr and others added 2 commits March 14, 2024 14:34
use unix epoc as modified time

Co-authored-by: Ramkumar Vengadakrishnan <ramkumarv@vmware.com>
this should makes the true diff in this PR easier to follow

Co-authored-by: Ramkumar Vengadakrishnan <ramkumar.vengadakrishnan@broadcom.com>
@crhntr crhntr marked this pull request as ready for review March 14, 2024 22:02
@crhntr crhntr requested a review from pabloarodas March 14, 2024 23:57
@pabloarodas pabloarodas merged commit 5552ac4 into main Mar 15, 2024
3 checks passed
@pabloarodas pabloarodas deleted the slingshots-bake-changes-02 branch March 15, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tas-slingshots Created by https://github.com/orgs/pivotal-cf/teams/tas-strategic-initiatives-slingshot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants