You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by dangogh <gi...@git.apache.org> on 2017/03/01 18:05:42 UTC
[GitHub] incubator-trafficcontrol pull request #327: [TC-72] Dockerfile build local
GitHub user dangogh opened a pull request:
https://github.com/apache/incubator-trafficcontrol/pull/327
[TC-72] Dockerfile build local
Reworked docker-compose.yml and Dockerfiles for component builds to use local directory rather than cloning from github.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dangogh/incubator-trafficcontrol dockerfile-build-local
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafficcontrol/pull/327.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #327
----
commit 62bd773320ccce50c105e4e51fe53c4c9cc7abd2
Author: Dan Kirkwood <da...@gmail.com>
Date: 2017-01-26T00:36:07Z
dockerfile for traffic_ops build from local dir
commit 0063ed392363d975bf773a95fcc13f1eb10ff65c
Author: Dan Kirkwood <da...@gmail.com>
Date: 2017-01-26T01:27:54Z
convert Dockerfile for TM build
commit 6f84674b4729115ffd6e849a99b14112c26560e6
Author: Dan Kirkwood <da...@gmail.com>
Date: 2017-01-26T03:34:58Z
convert Dockerfile for TR build
commit 8cfe075df15cc5da4de9d77306f0554ea7c6bb20
Author: Dan Kirkwood <da...@gmail.com>
Date: 2017-01-26T03:35:09Z
convert Dockerfile for TP build
commit 50f200c9d378f8f76db114934cc16c6edf69293f
Author: Dan Kirkwood <da...@gmail.com>
Date: 2017-01-26T03:35:44Z
convert Dockerfile for TS build
commit 5730e825206122eb1874f852fb16c8f7e71dda3e
Author: Dan Kirkwood <da...@gmail.com>
Date: 2017-01-26T03:36:18Z
run-build.sh not used any more..; no repo env vars needed
commit 37045649f114100934f7f61f5025787efff2be97
Author: Dan Kirkwood <da...@gmail.com>
Date: 2017-03-01T17:57:12Z
update BUILD.md
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Posted by alficles <gi...@git.apache.org>.
Github user alficles commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/327
This is a marked improvement, but it introduces a new issue: `make clean`. Since the build is happening directly in the local directories, all the temporary files will be left around. There's two ways around this, either copy the entire build folder into a new directory inside the container and just copy the final binaries out, or introduce a cleaning functionality that will remove the intermediate files.
I like the second option better, but the first is significantly easier, I think.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Posted by dangogh <gi...@git.apache.org>.
Github user dangogh commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/327
@rob05c -- `traffic_monitor` and `traffic_monitor_golang` both produce the same name rpm. Does the former need to be removed from the `docker-compose.yml`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Posted by alficles <gi...@git.apache.org>.
Github user alficles commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/327
+1. This is good to merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Posted by dangogh <gi...@git.apache.org>.
Github user dangogh commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/327
@alficles -- copying the repo wouldn't be easier: chicken vs egg. docker-compose operates from that directory, so would need to switch to the copy mid-stream. Cleaning up would be the better option, IMO. Main thing is rpmbuild dir. Some remnants left in TM and TR dirs. But any clean operation would need to avoid messing with other files *not* created by the build process...
Perhaps deleting the rpmbuild dir would be sufficient for now..
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Posted by alficles <gi...@git.apache.org>.
Github user alficles commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/327
Ok. I've tested this. It correctly produces binaries in the `dist` directory, without polluting your environment. For anyone else testing, make sure your docker is on the absolute latest. There's a bug in the penultimate build that is exercised by the `cp -a` in the new script.
I'm +1 on this merge, with a few comments:
- The `BUILD.md` file doesn't list the new tarball build option. This is a particularly useful option.
- The `BUILD.md` identifies the wrong target folder.
- The build command is really quite long. This is just an opinion, but we may want to add a tiny script that encodes the commands prescribed by `BUILD.md`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Posted by limited <gi...@git.apache.org>.
Github user limited commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/327
noted, will see how the merge looks. probably a go
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Posted by dangogh <gi...@git.apache.org>.
Github user dangogh commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/327
adding those 2 things to BUILD.md shortly.. For last point, I think deal with it separately. Created issue https://issues.apache.org/jira/browse/TC-180 for that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #327: [TC-72] Dockerfile build local
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafficcontrol/pull/327
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Posted by alficles <gi...@git.apache.org>.
Github user alficles commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/327
@dangogh @rob05c Yeah, `docker-compose up` should build all the things. As it stands, you'll get either a build failure or, worse, an unlabeled and arbitrary traffic_monitor binary in the output. I would strongly consider deciding which one is "mainstream" and either commenting out the build steps for the other, or reconfiguring the build to put the other's binaries in a different subfolder (which might be tricky).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Posted by dangogh <gi...@git.apache.org>.
Github user dangogh commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/327
that's what I needed to know.. thanks..
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Posted by dangogh <gi...@git.apache.org>.
Github user dangogh commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/327
@alficles -- latest should address those clean concerns..
@limited -- please consider pulling in to 2.0
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/327
@dangogh @alficles Yes, traffic_monitor and traffic_monitor_golang should
never both be built. One is replacing the other. The plan is to rename the
`traffic_monitor_golang` directory to `traffic_monitor` and remove the old
`traffic_monitor` once it's stable. Until we change the name and officially
"replace" the old one, Docker Compose shouldn't build
`traffic_monitor_golang`.
+1 on commenting it. Leaving in there and commented will make it easier for
those of us testing the new version.
On Fri, Mar 3, 2017 at 8:01 AM, alficles <no...@github.com> wrote:
> This is a marked improvement, but it introduces a new issue: make clean.
> Since the build is happening directly in the local directories, all the
> temporary files will be left around. There's two ways around this, either
> copy the entire build folder into a new directory inside the container and
> just copy the final binaries out, or introduce a cleaning functionality
> that will remove the intermediate files.
>
> I like the second option better, but the first is significantly easier, I
> think.
>
> \u2014
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/incubator-trafficcontrol/pull/327#issuecomment-283976269>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AE4GTdGNvkayhST2qZAkDrL6ZCbQRVukks5riCtJgaJpZM4MQARS>
> .
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Posted by alficles <gi...@git.apache.org>.
Github user alficles commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/327
Here's what I do on the ats-rpmbuild side:
```
#!/usr/bin/env bash
if [ ! -d /src ]; then
echo "Source directory does not exist."
exit 1
fi
cp -r /src /build
cd /build
autoreconf -vfi
./configure --prefix=/opt/trafficserver --enable-experimental-plugins --with-max-api-stats=4096
make DESTDIR=/artifacts -j install
```
So the idea is to mount the "live" folder, then copy it to a build folder, build, and copy back the relevant artefacts.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #327: [TC-72] Dockerfile build local
Posted by dangogh <gi...@git.apache.org>.
Github user dangogh commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/327
@limited -- is this what you had in mind? Perhaps should be considered for merge into 2.0? (for Apache compliance)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---