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

Adding gnoi test operations files #120

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

vamsipunati
Copy link
Contributor

BlackBoxTest represents a list of test APIs that facilitates testing the switch/network (injecting events, mutating switch state etc.). WhiteBoxTest represents a list of test APIs that facilitates testing the switch and controller connections.

@coveralls
Copy link

coveralls commented Apr 10, 2023

Pull Request Test Coverage Report for Build 4672800981

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 0.0%

Totals Coverage Status
Change from base Build 4579942267: 0.0%
Covered Lines: 0
Relevant Lines: 0

💛 - Coveralls

Copy link
Contributor

@marcushines marcushines left a comment

Choose a reason for hiding this comment

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

I think we want to add a markdown here as well to really start talking through the use cases of these api's

Also we should talk with the lemming team to provide lemming implementations of these services

Copy link
Member

@dplore dplore left a comment

Choose a reason for hiding this comment

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

Some comments to consider

// specified, verification runs for all valid components. It performs
// verification on the supplied components otherwise. RPC fails for any
// invalid component.
rpc VerifyState(VerifyStateRequest) returns (VerifyStateResponse);
Copy link
Member

Choose a reason for hiding this comment

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

This is similar to healthz.Check?

rpc Check(CheckRequest) returns (CheckResponse) {}

import "types/types.proto";


service BlackBoxTest {
Copy link
Member

Choose a reason for hiding this comment

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

Regardless could we make this more generalized for components? In the use case you describe, a transceiver is a type of component that we could reference.
https://github.com/openconfig/public/blob/66f4ae885b54270867ea942a7d30751e9029e101/release/models/platform/openconfig-platform-transceiver.yang#L36-L39

Something like
service ComponentSoftInstall
rpc Insert
rpc Remove
rpc state

or maybe
service ComponentProvision
rpc Provision
rpc UnProvision

I think this service is more likely a configuration, not a gNOI? This is because it is a stateful property being persisted on a device. It is not a "one shot" action that is invoked (like reboot).

// This RPC changes the state of the link connected to the specified port(s).
// The goal is to simulate a link going up/down due to factors external to the
// switch: copper cable/fiber removed, cable/fiber gone bad, etc.
rpc SetHardwareLinkState(SetHardwareLinkStateRequest)
Copy link
Member

Choose a reason for hiding this comment

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

I think setting link state is also a configuration, not a gNOI?

If the goal is to simulate 'loss of light' at an optical level, then we could add a leaf to the transceiver tree
something like
/openconfig-platform:components/component/openconfig-platform-transceiver:transceiver/physical-channels/channel/config/rx-laser-softdisable. When set to true, the laser receiver would be 'soft disabled', causing that's physical channel to be simulated as down.

Alternatively, this could also be done at higher layers 2 or 3 via /interfaces. This might be more simple and cover more use cases (being neutral to things like copper/optical and physical channels). But I am not sure if your use case requires L1 optical layer simulation.

repeated SetHardwareLinkStateStatus link_responses = 1;
}

message SetAlarmRequest {
Copy link
Member

Choose a reason for hiding this comment

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

FYI: I do think this qualifies as gNOI, because Alarms are not persistent state on a device.

However, should we instead pivot to healthz? Or does your use case require simulating already existing OC alarms?

Comment on lines +98 to +99
// Resource that raises the alarm. Must be a valid component ID.
string resource = 2;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Resource that raises the alarm. Must be a valid component ID.
string resource = 2;
// Component that raises the alarm. Must be a valid component ID. ie: /components/component/state/id
string component = 2;

// Resource that raises the alarm. Must be a valid component ID.
string resource = 2;
// Description of the alarm.
string description = 3;
Copy link
Member

Choose a reason for hiding this comment

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

Is the goal to set the alarm severity?


package gnoi.gnoi;

service WhiteBoxTest {
Copy link
Member

Choose a reason for hiding this comment

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

This could be achieved by configuration by configuring /system/control-plane-traffic/

However, another approach might be if your use case is to completely block a gRPC service for a fixed amount of time and then allow the DUT to restore it. You could define something like:

service SystemServices
// temporarily change the state of /system/services

message InterruptServiceState
string service_name = 1;
uint32 duration = 2; // msec of interruption

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.

4 participants