Zone manager basic implementation - improvements

[dteplits]

Cleaning code from unnecessary comments and debug printings Fixing bugs:

• fixing interface fqdn string
• handling properly IPv6 values

Adding input parameters validation Adding user custom fields handle

Unit tests will be added on a following PR

Signed-off-by: Diana Teplits dteplits@redhat.com

Release note:

NONE

[oshoval]

pull-kubesecondarydns-e2e-k8s not sure the lane is fine already, it doesn't run yet even on https://prow.ci.kubevirt.io

@AlonaKaplan fyi

if it does run and fails due to make create-nodeport there is a PR to fix it

maybe best to first push some commit to amend the PR so it will retrigger it else we might need to ask Daniel tomorrow

[oshoval]

/test pull-kubesecondarydns-e2e-k8s

[oshoval]

Now it does start the prow job https://prow.ci.kubevirt.io/view/gs/kubevirt-prow/pr-logs/pull/kubevirt_kubesecondarydns/24/pull-kubesecondarydns-e2e-k8s/1597482457127456768 (and passed)

but we need to see that it is also triggered automatically, and starts @dteplits can you please push an empty commit amend so we see the job is triggered ?

The dco is also hanged maybe the amend will fix those

EDIT - opened a dummy PR, it looks fine, thanks

[oshoval]

Please update PR desc with the needed info

[dteplits]

Please update PR desc with the needed info

done

[oshoval]

Fixing bugs - might be better please to elaborate a little what bugs are fixed (even briefly since those are the firsts PRs on the repo)

Please fix the release note

[dteplits]

Fixing bugs - might be better please to elaborate a little what bugs are fixed (even briefly since those are the firsts PRs on the repo)

Please fix the release note

fixed

[oshoval]

/release-note-none

[oshoval]

Thanks For some reason it didn't catch the release note (the above command fixes it) It should be like this, with the word None / NONE as content https://github.com/kubevirt/kubesecondarydns/pull/35/files#diff-18813c86948efc57e661623d7ba48ff94325c9b5421ec9177f724922dd553a35R26

[oshoval]

Was it tested that default namespace is also supported ?

[dteplits]

Was it tested that default namespace is also supported ?

yes

[oshoval]

Maybe the "vm" should be part of the A entry instead part of the ORIGIN ? I do understand it always exists, but the ORIGIN is more like the DOMAIN isn't it (which doesn't include the "vm")

on the other hand, we do need something to be the ORIGIN if domain is empty. and it is easier to have both cases something in common. (in this example DOMAIN = secondary.io)

EDIT - Anyhow its good as is, thanks

$ORIGIN vm.secondary.io. 
$TTL 3600 
@ IN SOA ns.vm.secondary.io. email.vm.secondary.io. (1 3600 3600 1209600 3600)
IN NS ns.vm.secondary.io.
IN A 127.0.0.1
nic1.vmi-sec.secondary-test IN A 10.10.0.5

When DOMAIN and NAME_SERVER_IP are empty strings:

$ORIGIN vm. 
$TTL 3600 
@ IN SOA ns.vm. email.vm. (1 3600 3600 1209600 3600)
nic1.vmi-sec.secondary-test IN A 10.10.0.5

[AlonaKaplan]

/approve

[AlonaKaplan]

/approve

[AlonaKaplan]

/approve

[kubevirt-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AlonaKaplan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubevirt/kubesecondarydns/blob/main/OWNERS)~~ [AlonaKaplan] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment