Network Port QoS Policy API

[Twister915]

Adding implementations for the following routes to the client would be desirable:

Specifically, these routes, under network/v2.0/qos (list here, as well: https://developer.openstack.org/api-ref/network/v2/index.html#quality-of-service)

QoS Extenstion common CRUD

• GET /rule-types [WIP] qos: list rule types #1030
• GET /rule-types/{rule_type} Networking V2: add QoS rule type Get call #1625
• GET /policies Networking V2: add QoS policies List #1591
• POST /policies Networking V2: add QoS policy Create call #1595
• GET /policies/{policy_id} Networking: add QoS policies Get, cleanup QoS doc #1593
• PUT /policies/{policy_id} Networking V2: add QoS policy Update call #1601
• DELETE /policies/{policy_id} Networking V2: add QoS policy Delete call #1602
• GET /policies/{policy_id}/bandwidth_limit_rules Networking V2: add bw limit rules List #1582
• POST /policies/{policy_id}/bandwidth_limit_rules Networking V2: add bw limit rule Create #1584
• GET /policies/{policy_id}/bandwidth_limit_rules/{rule_id} Networking V2: add bw limit rules Get #1583
• PUT /policies/{policy_id}/bandwidth_limit_rules/{rule_id} Networking V2: add bw limit rule Update #1588
• DELETE /policies/{policy_id}/bandwidth_limit_rules/{rule_id} Networking V2: add bw limit rule Delete #1590
• GET /policies/{policy_id}/dscp_marking_rules Networking V2: add QoS DSCP marking rules List #1594
• POST /policies/{policy_id}/dscp_marking_rules Networking V2: add QoS DSCPMarking rule Create #1604
• GET /policies/{policy_id}/dscp_marking_rules/{rule_id} Networking V2: add QoS DSCP rule Get call #1607
• PUT /policies/{policy_id}/dscp_marking_rules/{rule_id} Networking V2: add QoS DSCPMarking rule Update #1605
• DELETE /policies/{policy_id}/dscp_marking_rules/{rule_id} Networking V2: add QoS DSCP rule Delete call #1608
• GET /policies/{policy_id}/minimum_bandwidth_rules Networking V2: add QoS min bw rule List call #1613
• POST /policies/{policy_id}/minimum_bandwidth_rules Networking V2: add QoS min bw rule Create call #1615
• GET /policies/{policy_id}/minimum_bandwidth_rules/{rule_id} Networking V2: add QoS min bw rule Get call #1614
• PUT /policies/{policy_id}/minimum_bandwidth_rules/{rule_id} Networking V2: add QoS min bw rule Update call #1622
• DELETE /policies/{policy_id}/minimum_bandwidth_rules/{rule_id} Networking V2: add QoS min bw rule Delete call #1623

QoS Extension Ports CRUD

• GET /v2.0/ports/{port_id} Networking V2: add QoS policy extension for Port #1580
• POST /v2.0/ports/{port_id} Networking V2: add QoS policy extension for Port #1580
• PUT /v2.0/ports/{port_id} Networking V2: add QoS policy extension for Port #1580

QoS Extension Networks CRUD

• GET /v2.0/networks/{port_id} Networking V2: add QoS policy extension for Net #1581
• POST /v2.0/networks/{port_id} Networking V2: add QoS policy extension for Net #1581
• PUT /v2.0/networks/{port_id} Networking V2: add QoS policy extension for Net #1581

[ozerovandrei]

@jtopjian I have a question related to QoS support needed by terraform-provider-openstack#246.

Should we add the qos_policy_id port field into the Port structure or should we implement it as a separate extension similar to extradhcpopts?

If we add it as extensions how do we need to structure the package?

Should we add the extensions/qos/policies that will contain all logic for the qos-policies-qos with policies CRUD and port qos PortQoSPolicyOptsExt structure?
In that case we will also add NetworkQoSPolicyOptsExt to support the qos-policies-qos field for networks.

Or should we create the separate extensions/qos/portpolicies, extensions/qos/networkpolicies, etc.

Or it will be better if we just add the qos-policies-qos field into the existing Port structure?

[jtopjian]

Or it will be better if we just add the qos-policies-qos field into the existing Port structure?

Every once in a while I get the urge to combine all network extensions into their "base" resources. But then something comes along that makes me realize that's not a good idea. I recently looked at some DNS extensions which made me think we would run into problems when someone doesn't have the DNS extensions enabled.

The networking extensions are a bear to work with and I really want to simply them, but for now, we shouldn't do this.

If we add it as extensions how do we need to structure the package?

Should we add the extensions/qos/policies that will contain all logic for the qos-policies-qos with policies CRUD and port qos PortQoSPolicyOptsExt structure?
In that case we will also add NetworkQoSPolicyOptsExt to support the qos-policies-qos field for networks.

Yes. We can put everything in extensions/qos/policies/requests.go.

If the request opts for Ports and Networks are the same, unfortunately we will have to duplicate the structs. You can see an example of this with port security: https://github.com/gophercloud/gophercloud/blob/master/openstack/networking/v2/extensions/portsecurity/requests.go

But if the response fields are the same, we can cheat a little bit. Again, see port security: https://github.com/gophercloud/gophercloud/blob/master/openstack/networking/v2/extensions/portsecurity/results.go

Notice how there is only 1 struct, but we can use it for both:

• https://github.com/gophercloud/gophercloud/blob/master/acceptance/openstack/networking/v2/ports_test.go#L304-L307
• https://github.com/gophercloud/gophercloud/blob/master/acceptance/openstack/networking/v2/networks_test.go#L98-L101

Let me know if that helps.

[ozerovandrei]

@jtopjian thanks!

Is started playing with QoS and now I have more questions 😅

1 - Regarding qos_policy_id for the Port UpdateOpts.
How can we pass null as the string parameter into update request in case when we want to unset a QoS policy for a port?
It doesn't allow us to just pass an empty string.
I used commands from the config-qos#user-workflow, and got the following request:

{
    "port": {
        "qos_policy_id": null
    }
}

2 - Regarding design of the policies/rules CRUD.

We need to implement rule attribute of the policy object: #show-qos-policy-details.

To do this we must implement the rule object separately.

And the bad thing is that there are different rules objects with different URL paths and different attributes:

• qos-dscp-marking-rules;
• qos-minimum-bandwidth-rules;
• qos-bandwidth-limit-rules.

I guess that it's better to implement each rule in it's separate package. But how in that case we will be able to use slice of those different rules in the policy structure?

Can we do it with the structure composition? I mean can we implement a rule structure with common attributes across all rule types (id, tags) and then just use it across dscpmarkingrules, minimumbandwidthrules, bandwidthlimitrules packages?

[jtopjian]

How can we pass null as the string parameter into update request in case when we want to unset a QoS policy for a port?

You'll probably want to have the field type as:

QosPolicyID *string `json:"qos_policy_id,omitempty"`

Then when a user specifies an empty string, setopts["qos_policy_id"] = nil in the ToPortUpdateMap method. For example:

func (opts PortUpdateOptsExt) ToPortUpdateMap() (map[string]interface{}, error) {
  base, err := opts.UpdateOptsBuilder.ToPortUpdateMap()
  if err != nil {
    return nil, err
  }

  port := base["port"].(map[string]interface{})

  if opts.QoSPolicyID != nil {
    qosPolicyID := *(opts.QosPolicyID)
    if qosPolicyID != "" {
      port["qos_policy_id"] = qosPolicyID
    } else {
      port["qos_policy_id"] = nil
    }
  }

  return base, nil
}

I guess that it's better to implement each rule in it's separate package.

The different types of rules can be in the same package (rules) and you can have different CRUD functions and structs for each one (CreateDSCPMarkingRuleOpts, CreateDSCPMarkingRule(), etc).

However, for a policy's results, the rules field will have to be map[string]interface{}. I don't see any other way around this. In order to provide a typed set of rules, two things can be done:

The dev/user using Gophercloud can do it themselves:

for _, rule := range policy.Rules {
  ruleID := rule["id"].(string)
  ruleType := rule["type"].(string)

  switch ruleType {
  case "bandwidth_limit":
    typedRule, err := rules.GetBandWidthLimitRule(client, ruleID).Extract()
  }
}

or you can create a helper in gophercloud/utils:

for _, rule := range policy.Rules {
  typedRule, err := qos_utils.ConvertMapToRule(rule)
}

Where qos_utils.ConvertMapToRule is a function in gophercloud/utils which does something like the following:

switch rule["type"].(string) {
case "bandwidth_limit":
  typedRule := rules.BandwidthLimitRule{
    ID: rule["id"].(string),
    Type: rule["type"].(string),
    MaxKBPS: rule["max_kbps"].(int64),
  }
}

However, in above solution, typedRule will always be interface{} and the dev/user will always have to do a type assertion when working with the rules:

if v, ok := typedRule.(rules.BandwidthLimitRule); ok {
  ...
}

or policies/results.go could have a type like:

type BaseRule {
  ID string `json:"id"`
  Type string `json:"type"`
  Extra map[string]interface{}
}

And use the internal.RemainingKeys helper function. This will allow the common fields of id and type to be in an explicit field while putting all unique fields in Extra. Either of the two other solutions can still be done to create a fully typed field. However, this is getting a little complicated and I'd opt not to do this method - just have policies.Policy.Rules[] be []map[string]interface{}.

Unfortunately none of the solutions are great, but at least they are solvable :)

Let me know if this helps or if I'm misunderstanding something.

[ozerovandrei]

@jtopjian thank you, this is really good explanation. I think I will do it an add all of QoS stuff in terraform-provider-openstack in some near future.

[jtopjian]

@ozerovandrei Slight nit with the List action: how about we rename it to ListBandwidthLimitRules instead of BandwidthLimitRulesList? This will help match the format of the other actions (Get and Create).

[ozerovandrei]

@jtopjian sure

[ozerovandrei]

All done.

[jtopjian]

Nice work! 😄