-
Notifications
You must be signed in to change notification settings - Fork 178
Optionally set defaults file when minting images. #1864
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
base: main
Are you sure you want to change the base?
Conversation
22c93e9 to
62adee6
Compare
62adee6 to
bedf585
Compare
1176acf to
b573775
Compare
| return fmt.Errorf("install cuttlefish debs error: %v", err) | ||
| } | ||
| if *defaults_cuttlefish_integration_src != "" { | ||
| if err := gce.UploadFile(project, zone, insName, *defaults_cuttlefish_integration_src, "/etc/defaults/cuttlefish-integration"); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The image is created out of an attached disk that gets mounted in /mnt/image, the file should end up in /mnt/image/etc/defaults/cuttlefish-integration.
In this tool, the disk is mounted when executing the install.sh script in https://github.com/google/android-cuttlefish/blob/main/tools/baseimage/cmd/gce_install_cuttlefish_packages/main.go#L95.
After the installCuttlefishDebs call the disk is never unmounted, however, I'd recommend doing the copy to /mnt/image after the install.sh scripts is executed inside the installCuttlefishDebs function when the mounting happens, rather than relying on the hidden fact that installCuttlefishDebs mounts and but doesn't un-mount.
There's room for refactoring here, it can be done later thought.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to prepend /mnt/image.
This perhaps shouldn't be part of installCuttlefishDebs necessarily, because uploading this file isn't installing a package, but we can revisit later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's definitely not an ideal situation. I suggested to be part of installCuttlefishDebs for now because it's only within the context of that function where we make sure the attached disk is mounted on /mnt/image. Outside of the function, it's hacky to assume there's something mounted on /mnt/image.
b573775 to
5324cae
Compare
5324cae to
a829528
Compare
Then plumb that through to the script. This means that logic on the golang side owns the knowledge of the mountpoint, and doesn't need to assume that it knows what the mountpoint is from the shell script.
We want to ensure that static defaults are set when images run within particular environments, e.g., a particular GCE project. This occurs when there exists a file in `/etc/defaults/cuttlefish-integration` and contains `static_defaults_when=<condition>` (see google#1859). We thus need to ensure that the `cuttlefish-integration` file is set appropriately; we just do this by uploading the file when minting the image.
a829528 to
2cb5968
Compare
We want to ensure that static defaults are set when images run within particular environments, e.g., a particular GCE project. This occurs when there exists a file in
/etc/defaults/cuttlefish-integrationand containsstatic_defaults_when=<condition>(see #1859). We thus need to ensure that thecuttlefish-integrationfile is set appropriately; we just do this by a flag set appropriately when minting the image.