add support for setting SRVs for network #460
Conversation
|
Thx look good so far, i would need to have look more later. (Looked at it with phone). I think you missed the dpc part here https://github.com/dmacvicar/terraform-provider-libvirt/blob/master/website/docs/r/network.markdown |
|
@MalloZup updated to add docs in |
|
Globally looks good, thx!! I might have look on the comments on the vars , once i have time. E.g if there is nonot solution for that we might create issue here ans upstream |
| }, | ||
| "proto": { | ||
| Type: schema.TypeString, | ||
| // This should be required, but Terraform does validation too early |
There was a problem hiding this comment.
This is the same issue we faced when adding dns.hosts in #382
similar comment on https://github.com/dmacvicar/terraform-provider-libvirt/pull/460/files/de77bbb1d412cac90118176391ba9d349d1a38cc#diff-042e2374df248e4277a3506b065eb28aR174
i'm not 100% on terraform but, it looks like terraform complains in plan phase when the list from dns srv template has not been computed.
There was a problem hiding this comment.
Yop, i think we might have an issue just for remember it and fix it later. I will havw a look once i have terminal :)
There was a problem hiding this comment.
Yop, i think we might have an issue just for remember it and fix it later. I will havw a look once i have terminal :)
| Type: schema.TypeString, | ||
| Required: true, | ||
| }, | ||
| "proto": { |
There was a problem hiding this comment.
Why did you used proto when the libvirt equivalent is protocol?
There was a problem hiding this comment.
Oops i mxed up names from DNS SRV RFC 2782 and NetworkDNSSRV
fixed to match all names to NetworkDNSSRV
| Type: schema.TypeMap, | ||
| Computed: true, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Is there a reason some of the attributes present in NetworkDNSSRV are left out?
There was a problem hiding this comment.
Added the one missing field priority from NetworkDNSSRV
This commit adds a new template for generating dns srv records similar to dns hosts
and then adds `srvs` key to `dns` section for libvirt_network.
```hcl
a libvirt network DNS SRV template datasource
Datasource example:
data "libvirt_network_dns_srv_template" "etcd_cluster" {
count = "${var.etcd_count}"
service = "etcd-server"
protocol = "tcp"
domain = "${discovery_domain}"
target = "${var.cluster_name}-etcd-${count.index}.${discovery_domain}"
}
resource "libvirt_network" "k8snet" {
...
dns = [{
srvs = [ "${flatten(data.libvirt_network_dns_srv_template.etcd_cluster.*.rendered)}" ]
}]
...
}
```
|
@dmacvicar it would be awesome if you can take a second pass :). |
dmacvicar/terraform-provider-libvirt#460 is pending approval, so in the mean time changing the docs to point to forked repo for getting DNS SRV changes.
|
@abhinavdahiya thx ! |
This reverts commit 0443add. dmacvicar/terraform-provider-libvirt#460 was merged people still need to update their local builds..
|
I am getting this on the acceptance tests: Which is clearly missing quotes on: func TestAccLibvirtNetworkDataSource_DNSSRVTemplate(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: `data "libvirt_network_dns_srv_template" "etcd_cluster" {
count = 2
service = etcd-server-ssl
protocol = tcp
target = "my-etcd-${count.index}.tt.testing"
}`,
Check: resource.ComposeTestCheckFunc(I will push a fix, as it is blocking me from developing another branch. We need urgently to setup acceptance tests in CI somehow. |
|
@dmacvicar yop i agree with that. at least it can be also annoying if we need to run for each PR from contributors the testacc |
Fix broken acceptance tests from #460
This commit adds a new template for generating dns srv records similar to dns hosts
and then adds
srvskey todnssection for libvirt_network.