-
Notifications
You must be signed in to change notification settings - Fork 53
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
B-515: fix ignored vlan_id #561
base: master
Are you sure you want to change the base?
Conversation
This pull request is stale because it has been open for 30 days with no activity and it is not in a milestone. Remove 'status: stale' label or comment, or this will be closed in 5 days. |
This pull request is stale because it has been open for 30 days with no activity and it is not in a milestone. Remove 'status: stale' label or comment, or this will be closed in 5 days. |
if d.Get("automatic_vlan_id").(bool) { | ||
tpl.Add("AUTOMATIC_VLAN_ID", "YES") | ||
vlanSet = true | ||
} else if vlanid, ok := d.GetOk("vlan_id"); ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to OpenNebula docs on ovswitch vnets, automatic_vlan_id should be ignored in case vlan_id is defined (https://docs.opennebula.io/6.10/open_cluster_deployment/networking_setup/openvswitch.html#defining-open-vswitch-network). Meaning that, if both params are defined, vlan_id should have priority => vlan_id should be checked first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @dgarcia18 good catch! In any case, setting both attributes will raise a conflict error in the provider, but I've corrected it just in case and added a test for checking this case 👍🏼
"automatic_vlan_id": conflicts with vlan_id
Error: Conflicting configuration arguments
Signed-off-by: Aleix Ramírez <[email protected]>
hey @treywelsh I've added @dgarcia18 's suggestion and some tests in this PR: #581 Feel free to take a look and merge it to this branch, I think that with those changes this PR could be merged 👍🏼 Thanks! |
Community Note
Description
Allow to optionally set a
vlan_id
value for ovswitchRelated older PR #407
References
Close #515
New or Affected Resource(s)
Checklist
References
go fmt
)