Skip to content

Remove platform-specific Corefile templates#2410

Closed
cybertron wants to merge 1 commit intoopenshift:masterfrom
cybertron:de-dupe-corefile
Closed

Remove platform-specific Corefile templates#2410
cybertron wants to merge 1 commit intoopenshift:masterfrom
cybertron:de-dupe-corefile

Conversation

@cybertron
Copy link
Copy Markdown
Member

As far as I can tell, these are all providing the exact same records,
just using variations on the specific plugin. For the sake of simplicity,
let's converge on one Corefile template.

A couple of notes:

  1. I used the on-prem one because it handles ipv4 and ipv6 records
    in an RFC-compliant way, which none of the other plugins do.
  2. I left the kubevirt db file. I'm not sure why it's there since
    there doesn't seem to be a corresponding Corefile for the platform,
    and since I don't understand it I think it's safest to leave it
    alone.

- What I did

- How to verify it

- Description for the changelog

As far as I can tell, these are all providing the exact same records,
just using variations on the specific plugin. For the sake of simplicity,
let's converge on one Corefile template.

A couple of notes:
1) I used the on-prem one because it handles ipv4 and ipv6 records
   in an RFC-compliant way, which none of the other plugins do.
2) I left the kubevirt db file. I'm not sure why it's there since
   there doesn't seem to be a corresponding Corefile for the platform,
   and since I don't understand it I think it's safest to leave it
   alone.
@cybertron
Copy link
Copy Markdown
Member Author

/test e2e-openstack
/test e2e-ovirt
/test e2e-vsphere
/cc @mandre @Gal-Zaidman @jcpowermac @patrickdillon

@cybertron
Copy link
Copy Markdown
Member Author

/retest

metal-ipi failed to deploy the masters before this ever would come into play (and shouldn't be affected by this change anyway), openstack and vsphere both appear to have successfully deployed the nodes and failed on other things, which means coredns should be working. Unfortunately my must-gather patch hasn't merged yet so we don't have logs from the infra containers there yet.

@mandre
Copy link
Copy Markdown
Member

mandre commented Feb 15, 2021

/test e2e-openstack

@Gal-Zaidman
Copy link
Copy Markdown

@eslutsky can you please review this change ?

Copy link
Copy Markdown
Member

@mandre mandre left a comment

Choose a reason for hiding this comment

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

LGTM for OpenStack.

OpenStack was able to deploy the cluster which indicates DNS is working as expected. The job failed on an unrelated monitoring alert.

@kikisdeliveryservice
Copy link
Copy Markdown
Contributor

FYI: we now have owners files for template dirs so y'all will need to actually approve =)
see: #2410 (comment)

@mandre
Copy link
Copy Markdown
Member

mandre commented Feb 23, 2021

FYI: we now have owners files for template dirs so y'all will need to actually approve =)
see: #2410 (comment)

Great! in that case,

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 23, 2021
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cybertron, mandre
To complete the pull request process, please assign gal-zaidman, jcpowermac after the PR has been reviewed.
You can assign the PR to them by writing /assign @gal-zaidman @jcpowermac in a comment when ready.

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

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kikisdeliveryservice
Copy link
Copy Markdown
Contributor

/retest

@cybertron
Copy link
Copy Markdown
Member Author

/retest

C'mon openstack, you can do it! :-)

@mandre
Copy link
Copy Markdown
Member

mandre commented Mar 12, 2021

/test e2e-openstack

@cybertron
Copy link
Copy Markdown
Member Author

/retest

1 similar comment
@cybertron
Copy link
Copy Markdown
Member Author

/retest

@mandre
Copy link
Copy Markdown
Member

mandre commented Mar 18, 2021

FYI I'm moving openstack job to a more stable platform with openshift/release#16936

@cybertron
Copy link
Copy Markdown
Member Author

/retest

Looks like the openstack patch merged, so let's see if we have better luck now.

@romfreiman
Copy link
Copy Markdown

/test e2e-openstack

@mandre
Copy link
Copy Markdown
Member

mandre commented Mar 24, 2021

/retest

Looks like the openstack patch merged, so let's see if we have better luck now.

Sadly we experienced our first issue with Vexxhost right after we moved the openstack job there. It is fixed now.

/test e2e-openstack

@cybertron
Copy link
Copy Markdown
Member Author

/test e2e-openstack

Failed to bootstrap. Since bootstrapping is no longer dependent on DNS, I doubt it's related.

@cybertron
Copy link
Copy Markdown
Member Author

This went in as part of #2465 .

@cybertron cybertron closed this May 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants