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

Allow explicit dns configuration bug1160667 #456

Merged
merged 12 commits into from Oct 13, 2016
Merged

Allow explicit dns configuration bug1160667 #456

merged 12 commits into from Oct 13, 2016

Conversation

martin-mucha
Copy link

Changes proposed in this pull request:

  • feature page for "Allow explicit dns configuration bug1160667"

I confirm that this pull request was submitted according to the contribution guidelines: (please @mention yourself to sign)

This pull request needs review by: (please @mention the reviewer if relevant)

Martin Mucha added 2 commits September 1, 2016 13:38
initial commit with feature page layout for "Allow Explicit Dns
Configuration" feature.

Signed-off-by: Martin Mucha <mmucha@redhat.com>
@martin-mucha
Copy link
Author

@jhernand
@dankenigsberg
@mmirecki

please review.

</xs:extension>
</xs:complexContent>
</xs:complexType>
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The XML schema is a by-product of the specification of the API. If you use it here people may think that it is the primary mechanism to specify API types. I'd suggest you use an XML (or JSON) example instead, like you did below.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote this to explain, that there will be new element created. This element isn't defined by xml/json (however I can put example there), and most of our customers need not to understand our api-model. Therefore I put here xschema.

So to avoid confusion you mentioned — how about to putting here api-model code followed with info, that this definition will be during build expanded to this xschema? That way we're specifying here true element definition in both ways: our api-model and it's equivalent xschema.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you prefer. It is just that XML schema isn't how we specify the API and isn't how it is used. Both the actual specification and the XML example are better. The XML example is better for users, as they can use it directly. So I prefer the XML (or JSON) example, but this isn't that important.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add my vote for an xml snippet that an admin can use.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated.

  • used 'dns_resolver_configuration' name instead
  • removed 'ip' element usage in favor of new 'name_server' element
  • listed both model and XSchema definition with stress on fact, that XSchema is generated one.
  • added exemplary xml snippet

Martin Mucha added 2 commits September 13, 2016 14:31
Signed-off-by: Martin Mucha <mmucha@redhat.com>
Signed-off-by: Martin Mucha <mmucha@redhat.com>

## Summary
When a new host is added to the system, management network will be
created for it. As of ovirt-4.0.3, DNS configuration for such network
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about the more precise: "management network will be created for it" -> it would be attached to the management network
such network -> this network attachement

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a) actually if it is first host I believe the mgmt network will be created , but updated as requested.

b) if DNS configuration is not mentioned in engine backend, resolv.conf is used, but ONLY in VDSM. I have no instructions to obtain data from VDSM and store them into network attachment. (Have I?) Thus, with VDSM/host not having any network attachments, I do believe there should be 'network' and not 'network attachment'. Please confirm.

an admins should be able to specify overriding configuration in
WebAdmin. They can do that at several places:

* During creation new host
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we are dropping the first bullet.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

data will be passed to VDSM during SetupNetworks command and default
values from resolv.conf will be used. If either of them is used, it
will be passed to VDSM overriding configuration from resolv.conf. For
example we can set DNS configuration in Network related to certain DC
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

configuration in Network -> configuration in a network

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Two places will be altered:

* `network_attachments` table
* network table
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the same font for network table (to avoid confusion)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.


### REST

As mentioned, you can specify DNS Configuration at three places.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

three -> two

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

```

#### Updating 'network attachment' of Management Network on specific Host
only the network attachment of the management network can be updated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only->Only

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment is obsoleted by request to have dns configuration on any network. Done.


### GUI

As mentioned, you can specify DNS Configuration at three places:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

three->two

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


As mentioned, you can specify DNS Configuration at three places:

#### Creating new Host
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're dropping this section, I believe!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

![New Host Dialog with DNS Configuration](newHostDialogWithDnsConfiguration.png "New Host Dialog with DNS Configuration")

#### Updating Management Network
(note — when editing other network this option won't be available)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other->another

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Two places will be altered:

* `network_attachments` table
* network table
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly English nits, but a quite important change is needed all over the doc: we do not allow setting the DNS during add host flow. All references to this (including the image) should be dropped.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Martin Mucha added 2 commits September 15, 2016 10:31
call adding new host — this usecase is no longer supported.

fixed 'english' and rephrased some areas.

Signed-off-by: Martin Mucha <mmucha@redhat.com>
defined on management network will be applied during adding new host.

Signed-off-by: Martin Mucha <mmucha@redhat.com>
@martin-mucha
Copy link
Author

mburman asked question about supporting 'out of sync' for this. Do we want to support this?
If so, it means, that:

  • during creating new host DNS configuration must be copied from network to network_attachment
  • each network DNS configuration must be reported in getCaps call.

if we don't want to support that, it means, that:

  • after dns configuration is altered in engine, no further changes made in resolf.conf applies, and if they do (no idea how vdsm works here), it will be confusing for the engine user, since other value would be reported than one actually used.

You can update any network with DNS Configuration, however all such DNS
Configurations will be simply ignored during creation of new host except
for the DNS configuration defined on the network, which happens to be
management network at time of creating new host.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This paragraph does not belong here, but to the one above it. It seems to be speaking about networks and not about network attachments. I think that the former text is correct:
"Only the network attachment of the management network can be updated with DNS configuration."

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

![Edit Logical Network Dialog with DNS Configuration](editLogicalNetworkDialogWithDnsConfiguration.png "Edit Logical Network Dialog with DNS Configuration")

#### Updating 'attachment' of Management Network on specific Host
![Editing Network Attachment Dialog with DNS configuration](editNetworkAttachmentDialogWithDnsConfiguration.png "Editing Network Attachment Dialog with DNS configuration")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please add 'out-of-sync' scenario for manual changing of the resolv.conf under the engine's legs.
For example: Changing the name server in resolv.conf and then doing refresh caps in UI should end up in 'out-of-sync' management network. This should be applied also on multi-host command.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please check out newest commit.

Signed-off-by: Martin Mucha <mmucha@redhat.com>
As mentioned, you can specify DNS Configuration at two places:

#### Updating (Management) Network
You can update any network with DNS Configuration, however all such DNS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't like the idea to allow to update any logical network with DNS configuration.
It make no sense at all. Maybe we can find other solution on the host level, as DNS is a host property and not network.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discuss it with edward, who thinks opposite and is against what you've suggested.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not just "edward"; it's also the simplified API discussed with juan, as well as simplified user flows.

In reality, it is extremely rare to have one host with a specific DNS configuration. Most commonly, the DNS configuration is shared among all hosts in the subnet. Think about your DHCP-supplied DNS address, for example. We want to allow overriding the network-wide default on a specific host (via edit network), but this is for exceptional cases.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We (QE) are OK with this implementation.

Signed-off-by: Martin Mucha <mmucha@redhat.com>
@@ -40,11 +40,30 @@ values from resolv.conf will be used. If either of them is used, it
will be passed to VDSM overriding configuration from resolv.conf. For
example we can set DNS configuration in a Network related to certain DC
(or later to certain Cluster) and this configuration will override
configuration in resolv.conf. If we now setup configuration in
configuration in resolv.conf. Note: when Network is being attached in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd start a new paragraph with "Note"

when Network-> when a network

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

be used instead.
be used instead.

Copying dns configuration from Network record to NetworkAttachment is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dns->DNS (here and elsewhere in your current commit)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

configuration in resolv.conf. Note: when Network is being attached in
certain host, NetworkAttachment record is created for this, and
dns configuration is copied from Network record to NetworkAttachment
record. There we can further modify it. If we now setup configuration in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when the cluster-wide configuration changes? It would be nice if all hosts are updates (similarly to a change of vlan or label), except for hosts that have a local override.

I think that it would be better if we can avoid this copy until someone wants to override the cluster-wide configuration.

Please add a discussion about what happens when we change DNS config in the network while it is already the management network of multiple hosts.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright then:

  • no copying.
  • updating Network, which was already attached to some host, will make all it's relevant network attachments (which does not override it) out-of-sync. User can then click sync all network to reapply dns configuration.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In QoS, or VLAN we perform a multi-host action to update all attached hosts with the new value. Why not do it so with DNS as well? Even if the reason is the willingness to keep this feature small and simple, please state it in the feature page.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Martin Mucha added 2 commits September 22, 2016 11:12
• when updating Network, which was already attached to some host, will
make all it's relevant network attachments (which does not override it)
out-of-sync.

Signed-off-by: Martin Mucha <mmucha@redhat.com>
updated, except of those, which has this configuration overridden in
NetworkAttachment

Signed-off-by: Martin Mucha <mmucha@redhat.com>
@mykaul
Copy link

mykaul commented Sep 25, 2016

What happens with DHCP interfaces? Do we expect to override the DHCP settings or not?

wiki_category: Feature
wiki_title: Features/AllowExplicitDnsConfiguration
wiki_revision_count: 12
wiki_last_updated: 2015-10-14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Date

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

* Email: mmucha@redhat.com

## Summary
When a new host is added to the system, it would be attached to the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would -> is

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

• phrasing

Signed-off-by: Martin Mucha <mmucha@redhat.com>
@martin-mucha
Copy link
Author

Btw. quite essential question: what addresses are supported/wanted? IPV4 only or ? If both, we probably have to update design to mitigate Ipconfiguration/IpAddressAssignment in NetworkAttachment, so both can be used. Can someone advise?

@dankenigsberg
Copy link
Member

We need to add a section about DHCP: if the network is attached to a host via DHCP, it might get a conflicting DNS definition from there. Currently in Vdsm, we have PEERDNS=yes, which means that DHCP overwrites ovirt-side definition.

Signed-off-by: Martin Mucha <mmucha@redhat.com>
@martin-mucha
Copy link
Author

added note about dhcp. Feature page already contains info, that out-of-sync calculation might not be delivered in first version, so I did not think it's needed to add any more stress on that. What is supported and what not in this are probably should be noted in release notes, once feature is merged.

@dankenigsberg
Copy link
Member

@mmirecki we can merge the feature page as it is, as long as @mmucha-redhat remembers that out-of-sync should not be set if the host has dhcp configured on it.

@mmirecki mmirecki merged commit 4497669 into oVirt:master Oct 13, 2016
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

Successfully merging this pull request may close these issues.

None yet

7 participants