libvirt: Inject Wildcard dns records in host's dnsmasq

[gyohuangxin]

At this stage, we need a workaround to inject wildcard dns records in
host's dnsmasq, and domain entry in cluster-ingress-02-config.yml file should not contain cluster name, refer to
https://github.com/openshift/installer/blob/master/docs/dev/libvirt/README.md#console-doesnt-come-up.

We make it be done automatically at the installconfig-creating stage.

Fixes #1007

[openshift-ci-robot]

Hi @gyohuangxin. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gyohuangxin
To complete the pull request process, please assign sdodson
You can assign the PR to them by writing /assign @sdodson in a comment when ready.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[jichenjc]

@zeenix @jaypoulz maybe you can provide some comments on this PR? thanks a lot

[abhinavdahiya]

I don't think the installer should be using shell scripts in the asset generation esp when generating install-config.

If you want to do something when creating cluster that might still be debatable.

Why is this not something that the terraform or operator can't do?

Also don't change the ingress for all platforms.

[jichenjc]

agree we should only focus on libvirt only and don't affect others

when you say terraform you are suggesting that we use the terraform installer ? @abhinavdahiya
we had a long discussion here
openshift/cluster-ingress-operator#308

so the proposal is to generate a set of dns records and inject those records into dnsmasq configuration file or directly insert this into libvirt definition, both way we need the file like
following to be created

<hostname>oauth-openshift.apps.test1.aa.testing</hostname>
  <hostname>console-openshift-console.apps.test1.aa.testing</hostname>
  <hostname>downloads-openshift-console.apps.test1.aa.testing</hostname>
  <hostname>alertmanager-main-openshift-monitoring.apps.test1.aa.testing</hostnam
  <hostname>grafana-openshift-monitoring.apps.test1.aa.testing</hostname>
  <hostname>prometheus-k8s-openshift-monitoring.apps.test1.aa.testing</hostname>

[gyohuangxin]

@abhinavdahiya @jichenjc Thanks for your suggestions, and I also think it's a better way to do it in terraform or operator.

However, I was trying it in Ingress operator, refer to our discussion in openshift/cluster-ingress-operator#308, but it seems a hard work.

Why not do it in terraform?
Because it's a must to do echo -e "[main]\ndns=dnsmasq" | sudo tee /etc/NetworkManager/conf.d/openshift.conf and echo -e "server=/$domain/192.168.126.1\naddress=/.apps.$domain/192.168.126.51" | sudo tee /etc/NetworkManager/dnsmasq.d/openshift.conf before installing, otherwise, the installation will fail with an error like "cannot find xxxxx: no such host". Then, I drop the cluster name before generating configuration, which replaces doing $ sed -i 's/test1.//' $INSTALL_DIR/manifests/cluster-ingress-02-config.yml.

Also don't change the ingress for all platforms.

It only works on libvirt platform.

	if config.Platform.Libvirt != nil {
 		// Domain entry in cluster-ingress-02-config.yml file should not contain cluster name
 		domain = strings.Replace(domain, config.ObjectMeta.Name+".", "", 1)
 	}

Pls let me know if I missed something, Thanks.

[patrickdillon]

Considering that the installer is a binary, is this portable or does this expect hack/configure-dnsmasq.sh to be in the $PATH?

[jichenjc]

@zeenix @jaypoulz @gyohuangxin as another optinos. I created another one doing same thing here..
#2614

[jaypoulz]

@crawford @jichenjc @gyohuangxin

This looks like a good implementation (speaking purely in terms of getting dnsmasq to have the wildcard card), but I think @crawford should hold this PR for the same reason as #2614 (comment).

I will take this code and add it to the installation configuration for our CI job. Thanks again for for all the hard work in figuring this out.

[abhinavdahiya]

I don't think the current implementation of running the shell script is acceptable at all.

[jaypoulz]

@abhinavdahiya I believe you may have misunderstood my intention. This implementation is not acceptable as part of the installer. That much is clear. Please see the comment I have linked for context.

A similar implementation, however, can be used as a work around in the job configuration for our libvirt based CI job to get ingress traffic working. It won't be a perfect IPI solution, but until libvirt has support for injecting non-disruptive wildcard records - or someone comes up with a clean solution for a load-balancer as part of a libvirt deploy, we will need to work around this in a fashion similar to this.

I updated my comment to be a little less ambiguous and hope that this additional context is helpful.

[gyohuangxin]

@jaypoulz Thank you too.

I will take this code and add it to the installation configuration for our CI job. Thanks again for for all the hard work in figuring this out.

This code aims to make cluster installed and run well one time. Should it just generate a script so that CI job can use it configure dnsmasq locally or remotely?

And how do you take this code? Does a new branch need created?

Please tell me if there are any problems afterwards.

[jaypoulz]

@gyohuangxin I am working on a new branch of openshift/release that is going to live here while I work on it.

Basically, I will incorporate your shell script into a new test configuration template specific to our Z environment. I haven't gotten far enough yet to show you what I mean, but I will let you know if I have any trouble.

[abhinavdahiya]

using the local script to run dnsmasq updates is not acceptable and seems like we have moved to CI specific changes for now.

i'm closing this based on ^^

/close

Please feel free to reopen if you think I misunderstood the current state.

[openshift-ci-robot]

@abhinavdahiya: Closed this PR.

In response to this:

using the local script to run dnsmasq updates is not acceptable and seems like we have moved to CI specific changes for now.

i'm closing this based on ^^

/close

Please feel free to reopen if you think I misunderstood the current state.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.