frequenz-floss / frequenz-api-microgrid

gRPC+protobuf specification and Python bindings for the Frequenz Microgrid API
https://frequenz-floss.github.io/frequenz-api-microgrid/
MIT License
6 stars 6 forks source link

Add RPC to elevate role of core-apps if required #161

Open tiyash-basu-frequenz opened 11 months ago

tiyash-basu-frequenz commented 11 months ago

What's needed?

We may need a way to change the role of core-apps dynamically without requiring system downtime or manual intervention. A role elevation may be required in the future to set bounds on certain metrics, or to be able to execute certain RPCs (like start and stop).

This will act as a loose safeguard against clients accidentally/unintentionally setting bounds on critical metrics, and /or calling methods that could move a component to an unwanted state.

Proposed solution

This could be done using the following RPC:

service Microgrid {
...
  rpc AlterAppRole(AlterAppRoleRequest) returns (AlterAppRoleResponse);
...
}

message AlterAppRoleRequest {
  enum Role {
    ROLE_UNKNOWN = 0;
    ROLE_STANDARD = 1;
    ROLE_CORE = 2;
  }

  Role role = 1;
}

message AlterAppRoleResponse {
  enum RoleAlterStatus {
    ROLE_ALTER_STATUS_UNKNOWN = 0;
    ROLE_ALTER_STATUS_SUCCESS = 1;
    ROLE_ALTER_STATUS_FAILURE = 2;
  }

  RoleAlterStatus status = 1;
  google.protobuf.Timestamp valid_until_ts = 2;
}

This could also be done using proper authorization and authentication mechanisms to ensure that only authorized services/users can call this RPC.

Use cases

No response

Alternatives and workarounds

No response

Additional context

No response

thomas-nicolai-frequenz commented 11 months ago

I don't feel like this is the right approach to the problem. There are two things we want to achieve:

Step 1: To able to set bounds in very simple way ignoring any kind of priorities on whom is setting bounds.

service Microgrid {
  rpc SetComponentBounds(SetComponentBoundsRequest) returns (SetComponentBoundsResponse);
}

Step 2. We MIGHT want to be able to assign a priority to the bounds being set.

This part is up for discussion in my mind and highly uncertain. I'm not even sure its really needed tbh. The idea here is that we want core apps to have a higher priority on setting bounds as if some other app or actor would do it but I wonder if thats even needed or required. At least I can't tell this right now. Hence my suggestion would be to start with Step 1 and worry later about step 2 as this would almost require a logic to be implemented as in the SDKs PowerManager.

My suggestion would be to create a new issue for step 1 und close this issue here until we really have a requirement for that.

tiyash-basu-frequenz commented 11 months ago

This part is up for discussion in my mind and highly uncertain. I'm not even sure its really needed tbh.

I really do not think this is needed.

Hence my suggestion would be to start with Step 1

We already have RPCs for adding bounds.

My understanding after talking to you was that we needed to elevate the role of core-apps to get access to some features of the API (like setting bounds). If that is not the case, then we can close this issue :)

thomas-nicolai-frequenz commented 11 months ago

I really do not think this is needed.

Key point to me is that we do not really know right now with the knowledge and experience we have. Thus lets not decide wether we need it or not at this point in time and try to stay open minded about it.

We already have RPCs for adding bounds.

Oh wait, really? Damn. This might be where the none-alignment may have come from. My bad! Just checking.... I assume its these two, right:

I suggest to call them SetComponentXYZ instead of AddComponentXYZ because ADD implies there is also a REMOVE and I can't see any remove function.

tiyash-basu-frequenz commented 11 months ago

Key point to me is that we do not really know right now with the knowledge and experience we have. Thus lets not decide wether we need it or not at this point in time and try to stay open minded about it.

Noted.

I assume its these two, right:

  • AddComponentExclusionBounds
  • AddComponentInclusionBounds

I suggest to call them SetComponentXYZ instead of AddComponentXYZ because ADD implies there is also a REMOVE and I can't see any remove function.

The Microgrid API service functions by receiving bound parameters from multiple clients. However, it's important to note that clients do not establish fixed bounds for a given metric. Instead, they contribute a pair of bounds to an internally managed collection within the service. The service then calculates the final bounds through the intersection of inclusion bounds and the merger of exclusion bounds. As a result, the RPCs for this operation use the verb Add rather than Set. This terminology clarifies that clients are augmenting the existing set of bounds rather than assigning them to specific values.

thomas-nicolai-frequenz commented 11 months ago

The Microgrid API service functions by receiving bound parameters from multiple clients. However, it's important to note that clients do not establish fixed bounds for a given metric. Instead, they contribute a pair of bounds to an internally managed collection within the service. The service then calculates the final bounds through the intersection of inclusion bounds and the merger of exclusion bounds. As a result, the RPCs for this operation use the verb Add rather than Set. This terminology clarifies that clients are augmenting the existing set of bounds rather than assigning them to specific values.

OK, I hope thats clarified in the documentation (haven't checked it :-/) but it still begs the question how to remove a bound if it can be added. In that case I feel like neither Set nor Add does correctly describe the intention you laid out above.

This terminology clarifies that clients are augmenting the existing set of bounds rather than assigning them to specific values.

Maybe it should be called "AugmentComponentXYZBounds" instead.

tiyash-basu-frequenz commented 11 months ago

I hope thats clarified in the documentation

It is documented extensively in the RPCs and in the bounds definitions in the common specs, but could use another look.

it still begs the question how to remove a bound if it can be added.

Right, currently the service just removes the bounds after the timestamp specified in the return message. The service typically removes the bounds after 5 seconds. There is no way to cancel it now. Previously, clients used to stream bounds (using a client-side streaming RPC), so they could just stream a wider bounds object to override a previous tighter one. But when we changed to a simple-RPC, this feature got removed implicitly.

There should be a way to track each client individually in rust-gRPC services, so if/when we need, this feature can be brought back.

Maybe it should be called "AugmentComponentXYZBounds" instead.

Okay, will create an issue for it.