Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Allow rdgo logic to run local or in CI #114

Closed
wants to merge 1 commit into from

Conversation

ashcrow
Copy link
Member

@ashcrow ashcrow commented Jun 18, 2018

First pass. Needs work but I'd like to get at least @jlebon to take a gander and see where I'm mucking up in the Jenkinsfile :-).

The make target works. How it's wired in Jenkinsfile has not been tested.

@ashcrow ashcrow requested review from lucab and jlebon June 18, 2018 16:49
@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 18, 2018
dnf install -y git rsync openssh-clients dnf-plugins-core fedpkg dnf-utils awscli
cp $WORKSPACE/RPM-GPG-* /etc/pki/rpm-gpg/
dnf copr -y enable walters/buildtools-fedora
dnf install -y rpmdistro-gitoverlay
Copy link
Member

Choose a reason for hiding this comment

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

Feels like we should ultimately be modifying the coreos-assembler container to have these packages installed.

Copy link
Member Author

Choose a reason for hiding this comment

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

@cgwalters WDYT? ⬆️

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that's fine by me for now. Down the line we may want multiple containers though.

Copy link
Member

Choose a reason for hiding this comment

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


.PHONY: stage1
stage1:
docker run -ti --rm -e RDGO=${RDGO} -e WORKSPACE=${WORKSPACE} ${DOCKER_ARGS} ${DOCKER_IMG} /workspace/stages/stage1.sh
Copy link
Member

@miabbott miabbott Jun 18, 2018

Choose a reason for hiding this comment

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

s/docker/podman ? Ah, nevermind...this is going to be used by Jenkins too, so it is probably not podman aware

Copy link
Member Author

Choose a reason for hiding this comment

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

@miabbott for now that's correct.

@ashcrow
Copy link
Member Author

ashcrow commented Jun 18, 2018

@miabbott thanks for jumping on this and doing an initial review!

@cgwalters
Copy link
Member

Hmm, I don't think this is going to work - Jenkins does a lot of magic in talking to the docker socket to e.g. pass through $WORKSPACE as a hidden volume mount, etc.

@ashcrow
Copy link
Member Author

ashcrow commented Jun 18, 2018

@cgwalters we're defining $WORKSPACE ourselves via invocation in this PR. Are you saying that the $WORKSPACE variable will end up being overwritten by Jenkins?

@jlebon
Copy link
Member

jlebon commented Jun 18, 2018

Hmm, I was imagining the Makefile + Jenkinsfile combination to be less high-level. E.g. instead of wrapping the pipeline, it wraps steps inside some stages that'd make life easier (but assumes that all dependencies are already installed). This way it can be run inside the coreos-assembler container in CI as well as locally in pre-existing pet containers. For rdgo, we could have a make rdgo-build (or make rpms?) that just runs some idempotent setup goop + runs rdgo fetch && build. WDYT?

@jlebon
Copy link
Member

jlebon commented Jun 18, 2018

Are you saying that the $WORKSPACE variable will end up being overwritten by Jenkins?

It's not just the pwd. Environment variables, credentials, etc... Here's a sample from one of our builds where it invokes docker:

$ docker run -t -d -u 0:0 --net=host -v /srv:/srv --privileged -w /var/lib/jenkins/workspace/coreos-rhcos-cloud -v /var/lib/jenkins/workspace/coreos-rhcos-cloud:/var/lib/jenkins/workspace/coreos-rhcos-cloud:rw,z -v /var/lib/jenkins/workspace/coreos-rhcos-cloud@tmp:/var/lib/jenkins/workspace/coreos-rhcos-cloud@tmp:rw,z -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** quay.io/cgwalters/coreos-assembler cat

@ashcrow
Copy link
Member Author

ashcrow commented Jun 18, 2018

@jlebon yeah we can go lower level. I was hoping we wouldn't have to do so and would be able to have a clean separation between build and delivery. Do you see CI owning the container run and enter or should that be done by the build (IE: make)?

@jlebon
Copy link
Member

jlebon commented Jun 18, 2018

Hmm, thinking more on this, I'm not sure if this is the best approach to making the local dev case easier. If I look at the steps in #107 for example, those are still simpler than what the pipeline needs to do. E.g. now the cloud pipeline generates 4 cloud image variants using pipeline's parallel feature, which is cool. In local dev though, you probably just want a qcow2 you can boot locally. WDYT about just wrapping the steps in #107? So then folks can just do e.g.:

$ make rdgo
$ make treecompose
$ make qcow2

If this is about replicating the full Jenkins pipeline locally to hack on it; yeah, we definitely something for that. Though probably the easiest way to fix it would be to migrate to OpenShift and use a Jenkins template like we have in PACI: https://github.com/projectatomic/paci/tree/master/jenkins ?

Sorry for the flip on this! This is something I was interested in as well, though I'm unsure right now of the complexity of trying to unify the pipeline with local development.

@ashcrow
Copy link
Member Author

ashcrow commented Jun 18, 2018

Hmm, thinking more on this, I'm not sure if this is the best approach to making the local dev case easier. If I look at the steps in #107 for example, those are still simpler than what the pipeline needs to do. E.g. now the cloud pipeline generates 4 cloud image variants using pipeline's parallel feature, which is cool. In local dev though, you probably just want a qcow2 you can boot locally. WDYT about just wrapping the steps in #107? So then folks can just do e.g.:

$ make rdgo
$ make treecompose
$ make qcow2

I'm cool with that. The negative here is that if changes in the pipeline for rgdo, treecomposing, or creating images occur they should be reflected in the make file as well ... but I think we can live with that for the time being.

If this is about replicating the full Jenkins pipeline locally to hack on it; yeah, we definitely something for that. Though probably the easiest way to fix it would be to migrate to OpenShift and use a Jenkins template like we have in PACI: https://github.com/projectatomic/paci/tree/master/jenkins ?

I wasn't totally going in for replacing the pipeline in make but in trying to do something that allowed reuse for Jenkins and local devs I think it would have started to fall in to that.

Sorry for the flip on this! This is something I was interested in as well, though I'm unsure right now of the complexity of trying to unify the pipeline with local development.

It's OK! This is how we find out what works and what doesn't :-)

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 18, 2018
@ashcrow
Copy link
Member Author

ashcrow commented Jun 18, 2018

What I'll do then is take what's in #107 and turn that into make commands in a new PR.

@ashcrow ashcrow closed this Jun 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants