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.

Sign up for GitHub

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

Added the 'no_reboot` flag to the ActivateRequest message. #51

Merged
merged 2 commits into from
Apr 16, 2021
Merged

Added the 'no_reboot` flag to the ActivateRequest message. #51

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0bb0840
Added the 'activate_and_reboot` flag to the ActivateRequest message.
tomek-US Apr 13, 2021
ae27851
Changed 'activate_and_reboot' into 'no_reboot'.
tomek-US Apr 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
177 changes: 97 additions & 80 deletions os/os.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 12 additions & 4 deletions os/os.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import "github.com/openconfig/gnoi/types/types.proto";

option go_package = "github.com/openconfig/gnoi/os";

option (types.gnoi_version) = "0.1.0";
option (types.gnoi_version) = "0.2.0";
Copy link
Member

Choose a reason for hiding this comment

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

assuming the suggestion below, I am thinking this would be a micro bump and opposed to minor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


// The OS service provides an interface for OS installation on a Target. The
// Client progresses through 3 RPCs:
Expand Down Expand Up @@ -110,8 +110,9 @@ service OS {
rpc Install(stream InstallRequest) returns (stream InstallResponse);

// Activate sets the requested OS version as the version which is used at the
// next reboot, and reboots the Target. When booting the requested OS version
// fails, the Target recovers by booting the previously running OS package.
// next reboot, and reboots the Target if the 'activate_and_reboot' flag is
// set. When booting the requested OS version fails, the Target recovers by
// booting the previously running OS package.
rpc Activate(ActivateRequest) returns (ActivateResponse);

// Verify checks the running OS version. This RPC may be called multiple times
Expand Down Expand Up @@ -250,11 +251,18 @@ message InstallError {
// The ActivateRequest is sent by the Client to the Target to initiate a change
// in the next bootable OS version that is to be used on the Target.
message ActivateRequest {
// The version that is required to be activated and booted.
// The version that is required to be activated and optionally immediattely
// booted.
string version = 1;
// For dual Supervisors setting this flag instructs the Target to perform the
// action on the Standby Supervisor.
bool standby_supervisor = 2;
// If set to 'True' the Target will initiate the reboot process immediatelly
// after changing the next bootable OS version.
// If set to 'False' a separate action to reboot the Target and start using
// the activated OS version is required. This action CAN be executing
// the gNOI.system.Reboot() RPC.
bool activate_and_reboot = 3;
Copy link
Member

Choose a reason for hiding this comment

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

The inverse logic should be used here in order to keep the same behaviour with backward compatibility. Something like "no_reboot" or "activate_only" (proto3 defaults boolean to false when omitted). Where by default the Activate request will activate and reboot the target, unless this flag is set in which only activation will happen, but no reboot. This allows previous and actual versioned client and target implementations to coexist without a fundamental behaviour change of the RPC like it is being proposed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keeping backward compatibility sounds like good idea ;-) Done.

}

// The ActivateResponse is sent from the Target to the Client in response to the
Expand Down
Loading