CIAB: Fixed the method of obtaining IP by Shihta · Pull Request #3281 · apache/trafficcontrol

Shihta · 2019-01-30T06:33:04Z

What does this PR do?

The sequence of setting DNS record is shown as follows:

Set /etc/resolv.conf (infrastructure/cdn-in-a-box/dns/set-dns.sh)
Obtaining IP by "dig +short ${my_host}" (infrastructure/cdn-in-a-box/dns/insert-self-into-dns.sh)
Set DNS record (infrastructure/cdn-in-a-box/dns/insert-self-into-dns.sh)

It can not obtain IP after the /etc/resolv.conf changed.
This commit provide one way to fix it by changing the method of obtaining IP.
It uses parsing the ifconfig result instead of dig.

This commit also deletes insert-db-into-dns.sh.
DB should use insert-self-into-dns.sh instead of insert-db-into-dns.sh as well.

Beyond that, add trafficvault into the trafficops-perl dependency.
That could avoid the failure of obtaining trafficvault IP when trafficops-perl run set-to-ips-from-dns.sh.

Fixes #(issue_number)

Which TC components are affected by this PR?

What is the best way to verify this PR?

This is a bug fix, so just simply start CIAB and then check if http://video.demo1.mycdn.siab.test is available.

Check all that apply

This PR includes tests
This PR includes documentation updates
This PR includes an update to CHANGELOG.md
This PR includes all required license headers
This PR includes a database migration (ensure that migration sequence is correct)
This PR fixes a serious security flaw. Read more: www.apache.org/security

asfgit · 2019-01-30T06:36:10Z

Can one of the admins verify this patch?

dangogh · 2019-01-30T15:19:49Z

this won't fix the problem of trafficvault not being available for trafficops use -- it does not enforce startup order, only build dependencies.

That conflicts with one of the goals we had: ability to build and test trafficops in isolation.

Sorry that I didn't know it need the ability to build and test trafficops in isolation
I can remove it to meet this requirement.

But on the other hand, It does guarantee startup order of container.
Do you mean that this is not enough?
Should I add more steps in set-to-ips-from-dns.sh to guarantee that trafficvault registered its DNS record before set-to-ips-from-dns.sh add trafficvault IP to trafficcontrol/infrastructure/cdn-in-a-box/traffic_ops_data/servers/040-trafficvault_server.json?

adding TV as a dependency of TO guarantees that it will be started before TO, but that doesn't guarantee it will be done starting by the time TO needs it. "starting" the container is an instantaneous action that spins off the container's CMD as an asynchronous procedure. Ordering within that cannot be guaranteed. I think your idea about using the enroller could work, if I understand you right.

@ocket8888 You are right. that what I want to say.
@dangogh Since it need more work, I'll remove this line first and then send it in another PR.

Please refer to #3287 for this dependency problem

The sequence of setting DNS record is shown as follows: 1. Set /etc/resolv.conf (infrastructure/cdn-in-a-box/dns/set-dns.sh) 2. Obtaining IP by "dig +short ${my_host}" (infrastructure/cdn-in-a-box/dns/insert-self-into-dns.sh) 3. Set DNS record (infrastructure/cdn-in-a-box/dns/insert-self-into-dns.sh) It can not obtain IP after the /etc/resolv.conf changed. This commit provide one way to fix it by changing the method of obtaining IP. It uses parsing the ifconfig result instead of dig. This commit also deletes insert-db-into-dns.sh. DB should use insert-self-into-dns.sh instead of insert-db-into-dns.sh as well.

dangogh · 2019-02-12T15:37:13Z

ok to test

asfgit · 2019-02-12T16:01:49Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3214/
Test PASSed.

ghost · 2019-02-13T19:09:42Z

Hi @Shihta,

Thanks for submitting this PR:

I tested this PR and it appears some of the servers are still being added with the wrong subnet:

Problems I found:

The db, dns, enroller, and trafficvault servers are still being set with a 255.255.255.0 subnet.
The db and enroller servers also have empty ipAddress.

My suggestion is to delete all the files at infrastructure/cdn-in-a-box/traffic_ops_data/servers and have the server json payload generated dynamically. It doesn't make sense to keep static files around at this point in my opinion.

$ export CIAB_HOME="$HOME/src/trafficcontrol/infrastructure/cdn-in-a-box"
$ make very-clean
$ make -j 7
$ alias mydc="docker-compose "` \
    `"-f $CIAB_HOME/docker-compose.yml "` \
    `"-f $CIAB_HOME/docker-compose.expose-ports.yml "` \
    `"-f $CIAB_HOME/optional/docker-compose.socksproxy.yml "` \
    `"-f $CIAB_HOME/optional/docker-compose.socksproxy.expose-ports.yml "` \
    `"-f $CIAB_HOME/optional/docker-compose.vnc.yml "` \
    `"-f $CIAB_HOME/optional/docker-compose.vnc.expose-ports.yml "
$ mydc kill && mydc rm -fv && docker volume prune -f 
$ mydc build
$ mydc up -d

Wait for CiaB to fully start up and then get servers list from Traffic Ops API:

$ mydc exec trafficops /bin/bash
[root@trafficops app]# source /to-access.sh
[root@trafficops app]# to-get 'api/1.4/servers/' | jq '.response[] | { (.hostName):{"ipAddress":.ipAddress, "ipNetmask":.ipNetmask, "ipGateway":.ipGateway}}' | jq -s 'add'
{
  "trafficrouter": {
    "ipAddress": "172.25.0.14",
    "ipNetmask": "255.255.0.0",
    "ipGateway": "172.25.0.1"
  },
  "trafficmonitor": {
    "ipAddress": "172.25.0.9",
    "ipNetmask": "255.255.0.0",
    "ipGateway": "172.25.0.1"
  },
  "trafficops": {
    "ipAddress": "172.25.0.15",
    "ipNetmask": "255.255.0.0",
    "ipGateway": "172.25.0.1"
  },
  "trafficops-perl": {
    "ipAddress": "172.25.0.7",
    "ipNetmask": "255.255.0.0",
    "ipGateway": "172.25.0.1"
  },
  "db": {
    "ipAddress": "",
    "ipNetmask": "255.255.255.0",
    "ipGateway": "172.25.0.1"
  },
  "trafficportal": {
    "ipAddress": "172.25.0.8",
    "ipNetmask": "255.255.0.0",
    "ipGateway": "172.25.0.1"
  },
  "trafficvault": {
    "ipAddress": "172.25.0.11",
    "ipNetmask": "255.255.255.0",
    "ipGateway": "172.25.0.1"
  },
  "dns": {
    "ipAddress": "172.25.0.4",
    "ipNetmask": "255.255.255.0",
    "ipGateway": "172.25.0.1"
  },
  "enroller": {
    "ipAddress": "",
    "ipNetmask": "255.255.255.0",
    "ipGateway": "172.25.0.1"
  },
  "edge": {
    "ipAddress": "172.25.0.12",
    "ipNetmask": "255.255.0.0",
    "ipGateway": "172.25.0.1"
  },
  "mid": {
    "ipAddress": "172.25.0.10",
    "ipNetmask": "255.255.0.0",
    "ipGateway": "172.25.0.1"
  }
}

Shihta · 2019-02-14T03:02:17Z

@JBevillC thanks for helping me review it!

The 2 problems you found are not caused by this commit.
As you mentioned that there are some files in infrastructure/cdn-in-a-box/traffic_ops_data/servers.
They will be modified by infrastructure/cdn-in-a-box/traffic_ops/set-to-ips-from-dns.sh and then be sent by container enroller.

In fact, I have sent a PR #3264 trying to solve these 2 problems.
In PR #3264, it add a retry mechanism in obtaining IP and fix the netmask issue.
I didn't highlight it in slack because I'm not sure it's a right direction.

Should we just fix them by the PR #3264?
or should we let each service goes to enroll by themselves? like your opinion

It seems like the option 2 is better than 1.
If we want to select option 2, I'll close PR #3264 first and work the option 2 in the future.
On the other hand, you can take a look on PR #3287 as well. It's one of implementation of option 2 and aim to fix the sequence of service startup.

Back to this PR
It aim to fix the sequence of registering DNS record, not service enrollment of TO.
I don't know why, but registering DNS record of TO sometimes just fails in my environment.

The container enroller always shows:

+ nc -z trafficops.infra.ciab.test 443
trafficops.infra.ciab.test: forward host lookup failed: Unknown host
Waiting for https://trafficops.infra.ciab.test:443
+ echo 'Waiting for https://trafficops.infra.ciab.test:443'
+ sleep 5

as you can see, it shows forward host lookup failed: Unknown host

After check the container trafficops-perl log, I found:

+ insert-self-into-dns.sh
insert-self-into-dns domain ciab.test dns_key_path /shared/dns/Kciab.test.+157+11874.private my_host trafficops my_ip  my_fqdn trafficops.infra.ciab.test cmd 'update add trafficops.infra.ciab.test 86400 A '
could not read rdata
syntax error

as you can see, there is no ip addr

That why I sent this PR

ghost · 2019-02-15T03:09:52Z

@Shihta Thanks for the follow up. I'm going to re-test PRs #3264 and #3281 together (merged) against the latest master branch. Testing them both together seems like a better approach since they are highly related to the same area of CiaB DNS IP discovery and assignment. Thanks again for your contribution.

rob05c

Looks great, thanks!

Tested, CiaB comes up as expected, requesting the CiaB DS works as expected.

mitchell852 added the cdn-in-a-box related to the Docker-based CDN-in-a-Box system label Jan 30, 2019

dangogh reviewed Jan 30, 2019

View reviewed changes

Shihta force-pushed the Fixed_setting_DNS branch from 9ac7967 to e9a31e8 Compare January 31, 2019 01:20

rob05c approved these changes Feb 19, 2019

View reviewed changes

rob05c merged commit 475c5e1 into apache:master Feb 19, 2019

Shihta deleted the Fixed_setting_DNS branch February 26, 2019 01:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CIAB: Fixed the method of obtaining IP#3281

CIAB: Fixed the method of obtaining IP#3281
rob05c merged 1 commit into
apache:masterfrom
Shihta:Fixed_setting_DNS

Shihta commented Jan 30, 2019

Uh oh!

asfgit commented Jan 30, 2019

Uh oh!

dangogh Jan 30, 2019

Uh oh!

Shihta Jan 30, 2019

Uh oh!

ocket8888 Jan 30, 2019

Uh oh!

Shihta Jan 31, 2019

Uh oh!

Shihta Jan 31, 2019

Uh oh!

dangogh commented Feb 12, 2019

Uh oh!

asfgit commented Feb 12, 2019

Uh oh!

ghost commented Feb 13, 2019 •

edited by ghost

Loading

Uh oh!

Shihta commented Feb 14, 2019 •

edited

Loading

Uh oh!

ghost commented Feb 15, 2019

Uh oh!

rob05c left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

Shihta commented Jan 30, 2019

What does this PR do?

Which TC components are affected by this PR?

What is the best way to verify this PR?

Check all that apply

Uh oh!

asfgit commented Jan 30, 2019

Uh oh!

dangogh Jan 30, 2019

Choose a reason for hiding this comment

Uh oh!

Shihta Jan 30, 2019

Choose a reason for hiding this comment

Uh oh!

ocket8888 Jan 30, 2019

Choose a reason for hiding this comment

Uh oh!

Shihta Jan 31, 2019

Choose a reason for hiding this comment

Uh oh!

Shihta Jan 31, 2019

Choose a reason for hiding this comment

Uh oh!

dangogh commented Feb 12, 2019

Uh oh!

asfgit commented Feb 12, 2019

Uh oh!

ghost commented Feb 13, 2019 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Shihta commented Feb 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Feb 15, 2019

Uh oh!

rob05c left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ghost commented Feb 13, 2019 •

edited by ghost

Loading

Shihta commented Feb 14, 2019 •

edited

Loading