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

[launcher] Try to fix cloudbuild for launcher #458

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

jkl73
Copy link
Contributor

@jkl73 jkl73 commented Jun 1, 2024

Still experiencing empty files in OEM partition, try more logging and careful mount in the script.

Also moved cloudbuild.yaml from the root directory to launcher/ directory

@jkl73
Copy link
Contributor Author

jkl73 commented Jun 1, 2024

/gcbrun

Still experiencing empty files in OEM partition, try more logging
and careful mount in the script.
Move cloudbuild.yaml from root to launcher directory

Signed-off-by: Jiankun Lu <jiankun@google.com>
@jkl73
Copy link
Contributor Author

jkl73 commented Jun 4, 2024

/gcbrun

dynamic_substitutions: true
pool:
name: 'projects/confidential-space-images-dev/locations/us-west1/workerPools/cs-image-build-vpc'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove this private pool requirement, so people can submit the image build in their own project

Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we have this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used to have this to build images in the private pool. But I realize it's fine to have the image building step without private pool, just put tests in the private pool.

Good thing is people can now use this cloudbuild file directly to build image in their own project for testing and debugging

@jkl73 jkl73 requested a review from alexmwu June 4, 2024 19:58
dynamic_substitutions: true
pool:
name: 'projects/confidential-space-images-dev/locations/us-west1/workerPools/cs-image-build-vpc'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we have this?


# Now the oem partition is sealed, we mount it to print it's content
if [[ ! -d /mnt/disks/oem ]]; then
mkdir /mnt/disks/oem
Copy link
Contributor

Choose a reason for hiding this comment

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

Cleanup this directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is fine as this is not get persistent, just a tmp build environment

@@ -132,8 +132,12 @@ main() {
# Make sure cache is flushed for the OEM partition.
sync ${OEM_PATH}

# Remount as read-only to avoid unexpected changes
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this? Are you worried that some other thing is modifying the OEM partition after the fact? Wouldn't that get reflected in the dm-verity?

Copy link
Contributor Author

@jkl73 jkl73 Jun 4, 2024

Choose a reason for hiding this comment

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

yes, the behavior I saw is files are all good before the preload.sh finish, but somehow got corrupted before the dm-verity sealing step. So dm-verity looks all good, but the files are still corrupted.

Remount it as read-only (as it should be before this script) just to make sure it doesn't get overwritten accidentally

@jkl73 jkl73 merged commit f1be0a3 into google:main Jun 5, 2024
11 checks passed
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.

None yet

2 participants