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

vmware_dvs_portgroup: Add inherit attribute for DVS port groups #1316

Closed
mpl241 opened this issue May 9, 2022 · 5 comments
Closed

vmware_dvs_portgroup: Add inherit attribute for DVS port groups #1316

mpl241 opened this issue May 9, 2022 · 5 comments

Comments

@mpl241
Copy link

mpl241 commented May 9, 2022

SUMMARY

The module vmware_dvs_portgroup is missing attributes to allow a DVS port group to inherit settings from the parent switch. The vSphere API includes properties to allow DVS properties to inherit settings from the parent DVS. Looking at the MOB of a _vCenter 6.7 Update 3 system, the following DVS setting are inheritable:

config.defaultPortConfig.filterPolicy
config.defaultPortConfig.lacpPolicy
config.defaultPortConfig.inShapingPolicy
config.defaultPortConfig.ipfixEnabled
config.defaultPortConfig.macManagementPolicy
config.defaultPortConfig.macManagementPolicy.macLearningPolicy
config.defaultPortConfig.outShapingPolicy
config.defaultPortConfig.qosTag
config.defaultPortConfig.securityPolicy
config.defaultPortConfig.txUplink
config.defaultPortConfig.txUplink
config.defaultPortConfig.uplinkTeamingPolicy
config.defaultPortConfig.vendorSpecificConfig
config.defaultPortConfig.vlan
config.defaultPortConfig.vmDirectPathGen2Allowed

Some of these properties are mentioned in issue 1136, but that issue was opened another problem. Before I can successfully use this module to manage our DVS settings, this module must support the inherit properties for DVS port groups.

The default settings for this module should also be reviewed in light of the inherited settings. Overriding any of the inherit setting should be avoided unless a parameter is explicitly set by an attribute.

A good place to start in the VMware Web Services API guide to find these settings is here.

ISSUE TYPE
  • Feature Idea
COMPONENT NAME

vmware_dvs_portgroup

ADDITIONAL INFORMATION

Inheriting settings from the parent DVS switch simplifies the configuration of the DVS port groups, especially when managing many (50+) DVS port groups.

@mariolenz
Copy link
Collaborator

You're right, that's a problem. But I'm not sure what would be the best approach. Take, for example, config.defaultPortConfig.macManagementPolicy (config.defaultPortConfig.securityPolicy is more or less the same, although deprecated AFAIK) which is defined by the network_policy parameter of the module.

This parameter has default values. So it will always configure something. Now I wonder if we should introduce a parameter to ignore this and inherit the switch configuration, or if we should remove the defaults and set those configs to inherited when network_policy isn't defined.

You see, there are three problems when people don't define network_policy. It could mean:

  1. I don't care, leave it as it is.
  2. I don't care, just use the default.
  3. I don't care, the port group should simply inherit this from the switch.

At the moment, I tend to option 3. This would mean we don't need an explicit "inherit the switch configuration" parameter. But I fear some people might prefer option 1 :-/

@mpl241
Copy link
Author

mpl241 commented May 17, 2022

The modules should NOT configure something unless an attribute is passed to the module, inherited or not. I was not expecting any changes to network_policy because I didn't include it as an attribute. If the vSphere API requires an attribute, also make the module require the attribute. So, I think option 1 is the correct approach. Additionally, besides not enforcing a default, the module should provide a way to inherit a setting if the API allows.

Continuing with the network_policy example, the module provides a way to change the following API settings:

network_policy.promiscuous -> config.defaultPortConfig.macManagementPolicy.allowPromiscuous (boolean)
network_policy.forged_transmits -> config.defaultPortConfig.macManagementPolicy.forgedTransmits (boolean)
network_policy.mac_changes -> config.defaultPortConfig.macManagementPolicy.macChanges (boolean)

The module should be modified to allow the following setting, and should probably be mutually exclusive to the above settings, or maybe ignore the above settings.

network_policy.inherited -> network_policy,config.defaultPortConfig.macManagementPolicy.inherited (boolean)

@mariolenz
Copy link
Collaborator

Before I can successfully use this module to manage our DVS settings, this module must support the inherit properties for DVS port groups.

I doubt that you need support for all of them in order to use this module. That's a bit implausible, especially since three of the settings you mention are deprecated (lacpPolicy, qosTag and securityPolicy).

Could you define what you really need implemented to use this module? I mean really need, not just looking at the MOB / API and saying "all of this".

The default settings for this module should also be reviewed in light of the inherited settings. Overriding any of the inherit setting should be avoided unless a parameter is explicitly set by an attribute.

I've started to work on this in #1483. It's still work in progress, mind.

softwarefactory-project-zuul bot pushed a commit that referenced this issue Oct 9, 2022
…rom switch defaults (#1483)

vmware_dvs_portgroup : Remove defaults and add explicit inheritance from switch defaults

SUMMARY
Apart from the things that are specific to a portgroup (like, for example, its name) there are a lot of settings that could either be inherited from the switch or defined at the PG level.
Most, if not all, parameters of the module don't allow this: They have defaults which means the settings from the switch are always overwritten. Or they implicitly set something to inherited if the parameter is unset. This is a problem because people can't use the module in a way of "I don't care about this setting, just leave it as it is" manner or explicitly state that this setting should be inherited.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
vmware_dvs_portgroup
ADDITIONAL INFORMATION
#1316
@mariolenz
Copy link
Collaborator

Version 3.0.0 supports explicit inherit for at least some of the settings you've mentioned (#1483).

If that's not enough for you, please open a new (feature request) issue and tell us what's missing for your use case.

@mpl241
Copy link
Author

mpl241 commented Oct 10, 2022

@mariolenz Thanks for working on this request and sorry for the late reply. I will take a look at #1483 and open a new feature request if find the module is still missing something.

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

No branches or pull requests

2 participants