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] Change retry behavior to reboot #260

Merged
merged 1 commit into from
Nov 18, 2022
Merged

Conversation

jkl73
Copy link
Contributor

@jkl73 jkl73 commented Nov 12, 2022

  • Update/simplify scripts and service unit file
  • Update launcher directory, so main.go will be in launcher/launcher
  • add retryable/nonretryable/workload error, currently workload error is equivlent to retryable error

Signed-off-by: Jiankun Lu jiankun@google.com

@jkl73 jkl73 force-pushed the rebootretry branch 7 times, most recently from 035b8c8 to 3e86a63 Compare November 16, 2022 23:20
Copy link
Contributor

@alexmwu alexmwu left a comment

Choose a reason for hiding this comment

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

Mostly nits and style. The main issue I see is the complexity of getExitCode (I'd like to resolve this before more rounds of review).

@@ -0,0 +1,148 @@
// package main is a program that will start a container with attestation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just move this into the image subdirectory? launcher/launcher and the launcher/launcher/launcher binary is confusing

Copy link
Contributor

Choose a reason for hiding this comment

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

It also simplifies the go build step of the cloudbuild.

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 feel like it is okay,,, I want to separate the main file in its own directory, so the container_runner.go can be in its own package. The image folder mostly contains scripts and config, I don't think source code should be there.

launcher/image/preload.sh Outdated Show resolved Hide resolved
launcher/image/preload.sh Outdated Show resolved Hide resolved
launcher/image/exit_script.sh Outdated Show resolved Hide resolved
launcher/image/exit_script.sh Show resolved Hide resolved
launcher/container_runner.go Outdated Show resolved Hide resolved
launcher/image/exit_script.sh Show resolved Hide resolved
launcher/launcher/main.go Show resolved Hide resolved
launcher/launcher/main.go Show resolved Hide resolved
@jkl73 jkl73 force-pushed the rebootretry branch 2 times, most recently from 03da555 to 31dba6e Compare November 18, 2022 00:34
launcher/container_runner.go Outdated Show resolved Hide resolved
launcher/errors.go Outdated Show resolved Hide resolved
launcher/image/exit_script.sh Show resolved Hide resolved
launcher/launcher/main.go Show resolved Hide resolved
launcher/launcher/main_test.go Outdated Show resolved Hide resolved
Update scripts and service unit file

Signed-off-by: Jiankun Lu <jiankun@google.com>
@jkl73 jkl73 merged commit 3504e64 into google:master Nov 18, 2022
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request Dec 16, 2022
Breaking Changes:

New Features:
Add IsHardened in launch spec: google#244
Add container logging redirect policy: google#249
Add SEV-SNP attestation support: google#240
Integrity-protect stateful partition on CS image: google#251
Retry launcher OIDC token refresh with backoff: google#261
Change restart policy behavior to reboot: google#260
Add ability to GetGCEInstanceInfo from a certificate: google#267

Bug Fixes:
COS event log: require CEL events to use PCR13, add a launch separator, and don't skip unknown events: google#246
Measure LaunchSeparator event: google#247
Skip unallocated PCR selections when reading all PCRs: google#258
Remove gRPC client and use of insecure credentials: google#262
Fix server.VerifyAttestation proto merging(google#263) and defer of os.Exit(google#264): google#265

Other Changes:
Add fake verifier client: google#234
Update CI Go Version to 1.19: google#241
Add launcher integration testing support: google#255
Test multi-writer PD creation disabled: google#256
Update go-sev-guest dependency to v0.2.6: google#259
Change OIDC retry policy to hourly and add jitter to refresh time: google#266
Add wrapper cloudbuild workflow to trigger image build and testing: google#269
@alexmwu alexmwu mentioned this pull request Dec 16, 2022
alexmwu added a commit that referenced this pull request Dec 16, 2022
Breaking Changes:

New Features:
Add IsHardened in launch spec: #244
Add container logging redirect policy: #249
Add SEV-SNP attestation support: #240
Integrity-protect stateful partition on CS image: #251
Retry launcher OIDC token refresh with backoff: #261
Change restart policy behavior to reboot: #260
Add ability to GetGCEInstanceInfo from a certificate: #267

Bug Fixes:
COS event log: require CEL events to use PCR13, add a launch separator, and don't skip unknown events: #246
Measure LaunchSeparator event: #247
Skip unallocated PCR selections when reading all PCRs: #258
Remove gRPC client and use of insecure credentials: #262
Fix server.VerifyAttestation proto merging(#263) and defer of os.Exit(#264): #265

Other Changes:
Add fake verifier client: #234
Update CI Go Version to 1.19: #241
Add launcher integration testing support: #255
Test multi-writer PD creation disabled: #256
Update go-sev-guest dependency to v0.2.6: #259
Change OIDC retry policy to hourly and add jitter to refresh time: #266
Add wrapper cloudbuild workflow to trigger image build and testing: #269
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.

2 participants