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

[JENKINS-27152] Test of standardized temp directory with Git operations #81

Merged
merged 6 commits into from Mar 4, 2016

Conversation

@jglick
Copy link
Member

jglick commented Feb 29, 2016

jglick added 4 commits Feb 29, 2016
…InteractiveConsole: the screen recorder!

To suppress, add to ~/.m2/settings.xml:
<profile>
    <id>jenkins-acceptance-test</id>
    <properties>
        <JENKINS_WAR>/…/jenkins.war</JENKINS_WAR>
        <LOCAL_SNAPSHOTS>true</LOCAL_SNAPSHOTS>
        <RECORDER>off</RECORDER>
    </properties>
</profile>
and run with -Pjenkins-acceptance-test
@WithCredentials(credentialType=WithCredentials.SSH_USERNAME_PRIVATE_KEY, values={"git", "/org/jenkinsci/test/acceptance/docker/fixtures/GitContainer/unsafe"}, id="gitcreds")
@Issue("JENKINS-27152")
@Test public void sshGitInsideDocker() throws Exception {
// Pending https://github.com/jenkinsci/docker-workflow-plugin/pull/31 (and additional APIs in DockerFixture etc.)

This comment has been minimized.

Copy link
@jglick
@@ -158,4 +168,31 @@
build.shouldContainsConsoleOutput("divided into 3 sets");
}

@WithDocker
@WithPlugins({"workflow-aggregator@1.14", "docker-workflow@1.14-SNAPSHOT", "git", "ssh-agent@1.10-SNAPSHOT"})

This comment has been minimized.

Copy link
@jglick
@reviewbybees

This comment has been minimized.

Copy link

reviewbybees commented Feb 29, 2016

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

repo.transferToDockerContainer(host, port);
WorkflowJob job = jenkins.jobs.create(WorkflowJob.class);
job.script.set(
"node {ws('" + tmp.getRoot() + "') {\n" + // TODO UNIX_PATH_MAX workaround

This comment has been minimized.

Copy link
@jglick

jglick Feb 29, 2016

Author Member

Not happy about this but not sure what else to do @cyrille-leclerc: if you have a long workspace path, putting the SSH agent socket in or near it will not work, so the SSH Agent plugin will fall back to a global temp dir that will not be accessible from inside the Docker container.

Once jenkinsci/jenkins#2066 makes it into an LTS and we call it from both sides, that method could be adjusted to give an option to use a shortened path, say /tmp/<SHA-1-of-workspace>, which would be deterministic and unique, but nowhere near the workspace. Or /<slave-FS-root>/<some-hash>, to retain the property of being under the slave FS root and thus amenable to cleanup, bulk chmod operations, etc.?

This comment has been minimized.

Copy link
@recena

recena Mar 3, 2016

Contributor

@jglick I like this one /tmp/<SHA-1-of-workspace>

@jglick

This comment has been minimized.

Copy link
Member Author

jglick commented Mar 1, 2016

Note that the PR build in this case is useless since the test was skipped anyway (and the job is building against Jenkins 2.0 and failing massively).

"node {ws('" + tmp.getRoot() + "') {\n" + // TODO UNIX_PATH_MAX workaround
" docker.image('cloudbees/java-build-tools').inside {\n" +
" git url: '" + container.getRepoUrlInsideDocker() + "', credentialsId: 'gitcreds'\n" +
" sh 'mkdir ~/.ssh && echo StrictHostKeyChecking no > ~/.ssh/config'\n" +

This comment has been minimized.

Copy link
@recena

recena Mar 3, 2016

Contributor

I would add something like chmod 700 ~./ssh

This comment has been minimized.

Copy link
@jglick

jglick Mar 3, 2016

Author Member

There are no private files in this directory, only configuration, and anyway it is just a test.

@amuniz

This comment has been minimized.

Copy link
Member

amuniz commented Mar 3, 2016

🐝

BTW why is acceptance-test-harness job building against 2.0? There is a specific job for that.

@recena

This comment has been minimized.

Copy link
Contributor

recena commented Mar 3, 2016

LGTM 🐝

@jglick

This comment has been minimized.

Copy link
Member Author

jglick commented Mar 3, 2016

why is acceptance-test-harness job building against 2.0?

You would have to ask @olivergondza, I am not sure.

jglick added a commit that referenced this pull request Mar 4, 2016
[JENKINS-27152] Test of standardized temp directory with Git operations
@jglick jglick merged commit 6681f40 into jenkinsci:master Mar 4, 2016
1 check was pending
1 check was pending
Jenkins Jenkins is validating pull request ...
Details
@jglick jglick deleted the jglick:temp-dir-JENKINS-27152 branch Mar 4, 2016
@olivergondza

This comment has been minimized.

Copy link
Member

olivergondza commented Mar 4, 2016

why is acceptance-test-harness job building against 2.0?

We are copying the artifact from core/jenkins-core and in case it build RP against 2.0 we got these bits. ATH should run against master branch, ideally.

@amuniz

This comment has been minimized.

Copy link
Member

amuniz commented Mar 4, 2016

We are copying the artifact from core/jenkins-core

Then we need an specific job to build master (excluding PRs) and make ATH job to use that one. Should we move this conversation to the mailing list?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.