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.

Sign up for GitHub

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

Jump to bottom

for clients the network property "vrfshared" is always false #73

Open
mreiger opened this issue May 22, 2020 · 3 comments · May be fixed by #212
Open

for clients the network property "vrfshared" is always false #73

mreiger opened this issue May 22, 2020 · 3 comments · May be fixed by #212

Comments

@mreiger
Copy link
Contributor

mreiger commented May 22, 2020

When displaying a network with property vrfshared: true with metalctl network list --id=<network-id> -o yaml it falsely claims vrfshared is set to false:

$ metalctl network list --id=<network-id> -o yaml
- changed: "2020-05-08T05:37:46.487Z"
  created: "2020-03-10T14:22:21.823Z"
  description: ""
  destinationprefixes:
    - <...>
  id: <network-id>
  labels: {}
  name: <network name>
  nat: true
  parentnetworkid: null
  partitionid: ""
  prefixes:
    - <network prefix>
  privatesuper: false
  projectid: ""
  underlay: false
  usage:
      availableips: 16
      availableprefixes: 0
      usedips: 15
      usedprefixes: 0
  vrf: <vrf number>
  vrfshared: false
@mreiger
Copy link
Contributor Author

mreiger commented May 22, 2020

The culprit may also be https://github.com/metal-stack/metal-api.

@Gerrit91
Copy link
Contributor

Gerrit91 commented May 25, 2020 •
edited

Good finding. It's definitely coming from the metal-api because the vrfshared property does not get set in the network response. Therefore it comes with null for this property which the metalctl client always interprets as false.
It looks like we do not even persist this property in the database, we only use it when creating a new network to check whether it can have the same VRF ID like another network or not.

There are three possibilities to solve this, I assume:

  • Persist the property in the database
  • Dynamically find out the state of this property and add it to the response
  • Remove property from this response and only keep it in the network create request

@Gerrit91 Gerrit91 transferred this issue from metal-stack/metalctl May 25, 2020
@Gerrit91 Gerrit91 changed the title metalctl lies about network property vrfshared for clients the network property vrfshared in always false May 25, 2020
@Gerrit91 Gerrit91 changed the title for clients the network property vrfshared in always false for clients the network property "vrfshared" is always false May 25, 2020
@majst01
Copy link
Contributor

majst01 commented May 25, 2020

I think we can only decide between:

  • persist in the metal.Network entity
  • remove the property from the response

As this is only a flag for network creation and not used later in any case, i strongly vote removing this property from the response, and dont show it in the describe output.
Removing from the response is also more difficult, because we need to modify/split the NetworkImmutable struct therefore.

@Gerrit91 Gerrit91 linked a pull request Aug 10, 2021 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove vrfshared from network response. metal-stack/metal-api
3 participants
@majst01 @Gerrit91 @mreiger