search

kubevirt / kubesecondarydns

DNS for KubeVirt VirtualMachines secondary interfaces
Apache License 2.0
7 stars 8 forks source link

Filter out the default interface from the list passed to the ZoneManager #19

Closed AlonaKaplan closed 1 year ago

AlonaKaplan commented 1 year ago

This PR removes the TODOs from the main and controller files and actually creates and updates the zone manager. It also filters out the default interface from the list passed to the ZoneManager

Note: unit tests to interface_filter.go will be added on a following PR.

oshoval commented 1 year ago

Does it know to handle sriov with / without mac ? (without mac, the name won't exists so we can't create a FQDN for it)

AlonaKaplan commented 1 year ago

Does it know to handle sriov with / without mac ? (without mac, the name won't exists so we can't create a FQDN for it)

Good point, it doesn't. Let's open a ticket to think how to handle it. Maybe if the status doesn't contain name we should use the interfaceName from the status.

oshoval commented 1 year ago

Does it know to handle sriov with / without mac ? (without mac, the name won't exists so we can't create a FQDN for it)

Good point, it doesn't. Let's open a ticket to think how to handle it. Maybe if the status doesn't contain name we should use the interfaceName from the status.

ok about the ticket

It might conflict between vms even on the same cluster EDIT - not sure about same cluster as there is vm name included, but on two clusters - if they have the same DOMAIN then yes (but this can happen for other interfaces as well)

I think we should just ignore them, and have a note in the readme that mac is needed etc after all even the name is not supported if mac isnt once guest agent provides a solution we can adapt as well imo EDIT Another reason to ignore, is that the interfaceName is less predictable, so the user will need to get the oyaml find the right interface etc (vm logic might be needed as well for that) and just then able to use it

AlonaKaplan commented 1 year ago

Does it know to handle sriov with / without mac ? (without mac, the name won't exists so we can't create a FQDN for it)

Good point, it doesn't. Let's open a ticket to think how to handle it. Maybe if the status doesn't contain name we should use the interfaceName from the status.

ok about the ticket

It might conflict between vms even on the same cluster I think we should just ignore them, and have a note in the readme that mac is needed etc after all even the name is not supported if mac isnt once guest agent provides a solution we can adapt as well imo

I agree, it is written on the ticket.

oshoval commented 1 year ago

we need the 3rd commit as it blocks other PRs

zonemgr/zone_manager.go:26:14: Error return value of `zoneMgr.init` is not checked (errcheck)
    zoneMgr.init()
                ^