feature: implement grpc-client update#585
Conversation
bfjelds
changed the title
bfjelds
force-pushed
the
user/bfjelds/update-grpc-client
branch
from
9b10941 to
2de0634
Compare
bfjelds
force-pushed
the
user/bfjelds/update-grpc-client
branch
from
88df893 to
be989f2
Compare
bfjelds
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds support for performing A/B updates via the CLI gRPC client (trident grpc-client update) and updates Storm-based tests and test images to use that path where appropriate, including ensuring socket activation is enabled for the daemon.
Changes:
- Add/update gRPC client support for
Update(stage/finalize/unified) and adjust Trident’s update handling to allow finalize-only flows without requiring a new host config. - Update Storm rollback/e2e/servicing helpers to invoke
trident grpc-client updatedepending on runtime/test scenario. - Enable
tridentd.socketin multiple VM test image YAMLs to support gRPC socket activation.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/storm/servicing/tests/update.go | Switches servicing update commands to trident grpc-client update for stage/finalize calls. |
| tools/storm/rollback/tests/helper.go | Uses trident grpc-client update for rollback update path and improves command logging. |
| tools/storm/helpers/ab_update.go | Chooses update vs grpc-client update based on runtime type when invoking update. |
| tools/storm/e2e/scenario/ab_update.go | Chooses update vs grpc-client update based on runtime type for e2e A/B update flows. |
| tools/pkg/tridentgrpc/grpc.go | Adds UpdateServiceClient to the Go gRPC client wrapper. |
| tests/images/trident-vm-testimage/base/updateimg-grub.yaml | Enables tridentd.socket on VM test images. |
| tests/images/trident-vm-testimage/base/updateimg-grub-verity.yaml | Enables tridentd.socket on VM test images. |
| tests/images/trident-vm-testimage/base/updateimg-grub-verity-azure.yaml | Enables tridentd.socket on VM test images. |
| tests/images/trident-vm-testimage/base/baseimg-usr-verity.yaml | Enables tridentd.socket on VM test images. |
| tests/images/trident-vm-testimage/base/baseimg-root-verity.yaml | Enables tridentd.socket on VM test images. |
| tests/images/trident-vm-testimage/base/baseimg-grub.yaml | Enables tridentd.socket on VM test images. |
| tests/images/trident-vm-testimage/base/baseimg-grub-verity.yaml | Enables tridentd.socket on VM test images. |
| tests/images/trident-vm-testimage/base/baseimg-grub-verity-azure.yaml | Enables tridentd.socket on VM test images. |
| crates/trident/src/lib.rs | Allows finalize-only update to use stored HostStatus spec when no new host config is provided. |
| crates/trident/src/grpc_client/tridentclient.rs | Implements update RPC calls (unified, stage-only, finalize-only) in Rust gRPC client. |
| crates/trident/src/grpc_client/mod.rs | Wires grpc-client update CLI command to the new update RPC methods and reads config input. |
| // Stable APIs | ||
| tridentpbv1.VersionServiceClient | ||
| tridentpbv1.StreamingServiceClient | ||
| tridentpbv1.UpdateServiceClient | ||
| grpcConn *grpc.ClientConn |
There was a problem hiding this comment.
UpdateServiceClient is added to TridentClient but NewTridentClientFromNetworkConnection doesn’t populate it, leaving the embedded interface nil; any use of the update RPCs through this client will fail at runtime. Initialize UpdateServiceClient in the constructor alongside VersionServiceClient and StreamingServiceClient.
There was a problem hiding this comment.
i think that's ok for now? nothing should be using it yet
| updateCmd := fmt.Sprintf("sudo trident -v trace grpc-client update %s", vmHostConfigPath) | ||
| logrus.Tracef("Invoking `trident update` on VM: '%s'", updateCmd) | ||
| updateOutput, err := stormssh.SshCommandCombinedOutput(u.VMConfig.VMConfig, u.VMIP, updateCmd) |
There was a problem hiding this comment.
The trace log message says Invoking trident update`` but the command being executed is trident ... grpc-client update, which can be misleading when debugging failures. Update the log message (and the later "`trident update` invoked" message) to match the actual command being run.
| logrus.Tracef("Running Trident update staging command on VM") | ||
| combinedStagingOutput, stageErr := stormssh.SshCommandCombinedOutput(vmConfig.VMConfig, vmIP, fmt.Sprintf("sudo trident update %s %s --allowed-operations stage", tridentLoggingArg, updateConfig)) | ||
| combinedStagingOutput, stageErr := stormssh.SshCommandCombinedOutput(vmConfig.VMConfig, vmIP, fmt.Sprintf("sudo trident grpc-client update %s %s --allowed-operations stage", tridentLoggingArg, updateConfig)) | ||
| if testConfig.Verbose { |
There was a problem hiding this comment.
PR description says servicing tests (and tests that split stage/finalize) should keep using trident update, but this test now invokes trident grpc-client update for separate stage and finalize calls. Either update the PR description to match this behavior or revert this test to trident update if split update via gRPC client isn’t intended here.
|
/AzurePipelines run [GITHUB]-trident-pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
🔍 Description
grpc-client updatetrident grpc-client update. For container tests and servicing tests and e2e tests where stage and finalize are separate update calls,trident updatewill remain.Validation