[JENKINS-39370,JENKINS-39369] - Support of work directories in Remoting #129

oleg-nenashev · 2016-10-31T11:54:02Z

This pull requests adds a basic support of workspaces to Remoting. It offers options, which allow enabling workspaces for JNLP and SSH agents in Jenkins.

In the original implementation we had no way to determine working directory on startup. Remoting itself didn't know about workspaces even if it was a Jenkins agent/slave
With previous parameters the remoting behavior does not change
The change will require additional configuration on the Jenkins side to enable workspaces by default

Scope of changes:

Introduce concept of working directory in the remoting (should be compatible with Jenkins agents) - JENKINS-39370
Automatically create STDERR and STDOUT outputs if the workspace is enabled - JENKINS-39369
Use alternate JAR cache location if workspace is specified JENKINS-18578
Add option to fail the agent startup if the workdir does not exist JENKINS-39130
Functional tests
Automatic Logrotate for logs within workdir. Should be manageable by JUL settings
Documentation
Adjust logging to make it compliant with docs, add support of JUL property files

Potential improvements:

Postponed: specifying workdir after the agent startup (let Jenkins enable it?)
TBD: other magic to automagically determine Workdir in a compatible way

…ly create logs there if specified

batmat · 2016-11-02T08:14:59Z

src/main/java/hudson/remoting/Launcher.java

+
+        if (slaveLog != null) { // Legacy behavior
+            System.out.println("Using " + slaveLog + " as an agent Error log destination. 'Out' log won't be generated");
+            System.out.flush(); // Just in case the channel


missing words/sentence end in the comment?

batmat · 2016-11-02T08:16:00Z

src/main/java/hudson/remoting/Launcher.java

+                if (!workDir.isDirectory()) {
+                    throw new IOException("The specified agent working directory path points to a non-directory file");
+                }
+                if (!workDir.canWrite() || !workDir.canRead() || !workDir.canExecute()) {


Should/couldn't it just (try to) create it? Like for .jenkins will just be created if absent?

It creates it on-demand. This code is being executed only for existing files

batmat · 2016-11-02T08:22:07Z

src/main/java/hudson/remoting/Launcher.java

+            System.setErr(new PrintStream(new TeeOutputStream(System.err,
+                    new FileOutputStream(new File(internalDirPath.toFile(), "remoting.err.log")))));
+            System.setOut(new PrintStream(new TeeOutputStream(System.out,
+                    new FileOutputStream(new File(internalDirPath.toFile(), "remoting.out.log")))));


I'm wondering if this wouldn't be easier, for diagnostics purposes, to just output in a single file, and use JUL (I don't like it very much, but as it's already present in the JDK and Jenkins code already uses it in general...) or another logging API to distinguish between messages severities. E.g. here we don't have the dates of the logs, so it would make btw it hard or impossible to correlate .err logs to .out ones, right?

I was considering it, but unfortunately SOME of executions on agents in plugin implementations write to STDOUT/STDERR instead of using JUL or whatever other logging. Hence we have to maintain such low-level API. Otherwise I would just implement JUL appender

…ting work directory

… workdir is missing

oleg-nenashev · 2016-11-16T23:01:54Z

src/main/java/hudson/remoting/Engine.java

+     */
+    public synchronized void startEngine() throws IOException {
+        // Prepare the working directory if required
+        final Path path = WorkDirManager.getInstance().initializeWorkDir(workDir.toFile(), internalDir, failIfWorkDirIsMissing);


Ideally should be moved to /remoting/logs and logrotated

oleg-nenashev · 2016-11-17T15:26:55Z

Tests pass for me locally. I have no idea how these changes cause such failure in the Channel property. And #132 works like a charm

…j IDEA adoption fun

oleg-nenashev · 2016-11-27T22:19:27Z

@reviewbybees would be useful

stephenc

Looks mostly ok, I think you can use the aliases for slaveLog and rename it now. I also wonder if you have missed passing slaveLog through

stephenc · 2016-12-09T12:59:14Z

src/main/java/hudson/remoting/Launcher.java

+     * If specified, this option overrides the default destination within {@link #workDir}.
+     * If both this options and {@link #workDir} is not set, the log will not be generated.
+     */
+    @Option(name="-slaveLog", usage="Local agent error log destination (overrides workDir)")


@Option(name="-agentLog", aliases={"-slaveLog"}, ... and be done with the rename

I was considering renaming.
My main concern about agent is that we will have Jenkins agent and remoting agentterms. And they won't be equal in general.

stephenc · 2016-12-09T13:00:36Z

src/main/java/hudson/remoting/Launcher.java

+                jnlpArgs.add(internalDir);
+                jnlpArgs.add("-failIfWorkDirIsMissing");
+                jnlpArgs.add(Boolean.toString(failIfWorkDirIsMissing));
+            }
            if (candidateCertificates != null && !candidateCertificates.isEmpty()) {


pass through slaveLog/AgentLog?

There is https://issues.jenkins-ci.org/browse/JENKINS-39817 for it. I'm not a big fan of adding this option since we are going to have the workDir now. But maybe it's good for the code consistency

rsandell · 2016-12-13T14:40:52Z

src/main/java/org/jenkinsci/remoting/engine/WorkDirManager.java

+     * @throws IOException Verification failure
+     */
+    private static void verifyDirectory(@Nonnull File dir, @Nonnull DirType type, boolean failIfMissing) throws IOException {
+        if (dir.exists()) {


Hmm, I would expect the process to try to create the directory path if it is missing and complain if it can't create the directory.

rsandell · 2016-12-13T14:41:39Z

src/main/java/org/jenkinsci/remoting/engine/WorkDirManager.java

+
+    /**
+     * Sets up logging subsystem in the working directory.
+     * It the logging system is already initialized, do nothing


oleg-nenashev · 2017-02-17T08:05:26Z

@reviewbybees No additional reviews required right now. Will push the logrotate support soon as we have discussed with @stephenc

jglick · 2017-02-17T14:21:27Z

JENKINS-18578 seems like the most critical change.

@stephenc

@stephenc suggested doing it in the PR, so I decided to address it as a part of JENKINS-39370. But the code still has initialization in hudson.remoting.Launcher for other logging modes.

…JENKINS-39370

…in its interbal directory

…cher

oleg-nenashev · 2017-05-03T14:25:31Z

@reviewbybees I am working on extra functional tests, but the core logic & documentation is ready to review

ghost · 2017-05-03T14:49:12Z

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.

…ogging.config.file system property

oleg-nenashev · 2017-05-04T12:14:19Z

@reviewbybees Tests are also ready, so it can be reviewed. I will post the design doc by tomorrow.

stephenc

Changes since my last revew LGTM

oleg-nenashev · 2017-05-07T07:51:39Z

@reviewbybees done. I will do more testing in downstream PRs

[JENKINS-39370,JENKINS-39369] - Add workDir parameter and automatical…

d0abdf6

…ly create logs there if specified

oleg-nenashev added the work-in-progress label Oct 31, 2016

batmat reviewed Nov 2, 2016

View reviewed changes

oleg-nenashev added 5 commits November 9, 2016 13:11

Save the progress

c657661

[JENKINS-39370] - Generalize the workspace manager for Java Web Start

48fe307

[JENKINS-39370] - WiP - Save progress in the test suite

ece2848

[JENKINS-39370] - Add tests for WorkDirManager

8550a79

[JENKINS-39370] - Restrict the range of supported symbols in the remo…

af01961

…ting work directory

oleg-nenashev changed the title ~~[JENKINS-39370,JENKINS-39369] - Improve logging of agents~~ [JENKINS-39370,JENKINS-39369] - Support of working directories in Remoting Nov 16, 2016

oleg-nenashev added 3 commits November 16, 2016 23:42

[JENKINS-39370] - Generalize the workspace initialization checks

ecc845f

[JENKINS-39130] - Allow specifying flag for failing initialization if…

374f423

… workdir is missing

[JENKINS-39370] - Reference DirType in WorkDirManager Javadocs

7bcbff4

oleg-nenashev commented Nov 16, 2016

View reviewed changes

oleg-nenashev closed this Nov 17, 2016

oleg-nenashev reopened this Nov 17, 2016

oleg-nenashev closed this Nov 17, 2016

oleg-nenashev reopened this Nov 17, 2016

oleg-nenashev added 4 commits November 17, 2016 17:04

[JENKINS-39370] - @stephenc noticed that workDir may be null, Intelli…

aa10398

…j IDEA adoption fun

[JENKINS-39370] - Seems this message breaks our CI

ac05a22

[JENKINS-39370] - Another message, which likely breaks the CI instance

b465606

[JENKINS-39370] - Simplify the log handling logic

9dc3559

oleg-nenashev added the needs-review label Dec 9, 2016

stephenc reviewed Dec 9, 2016

View reviewed changes

rsandell reviewed Dec 13, 2016

View reviewed changes

oleg-nenashev mentioned this pull request Jan 31, 2017

Print the FS Jar Cache directory in exception when it is not writable #143

Merged

oleg-nenashev removed the needs-review label Feb 17, 2017

oleg-nenashev added 3 commits February 23, 2017 13:42

[JENKINS-39817] - Introduce the agentLog parameter in remoting.jnlp.Main

fe336a2

@stephenc suggested doing it in the PR, so I decided to address it as a part of JENKINS-39370. But the code still has initialization in hudson.remoting.Launcher for other logging modes.

Enable JUL logging to a log-rotated file by default

4b7ee11

Merge commit '9e2f16ade84a841f8d32ed1a4b5ad3c8ca4cb141' into feature/…

813135f

…JENKINS-39370

oleg-nenashev changed the title ~~[JENKINS-39370,JENKINS-39369] - Support of working directories in Remoting~~ [JENKINS-39370,JENKINS-39369] - Support of work directories in Remoting May 2, 2017

oleg-nenashev added 6 commits May 2, 2017 14:03

Merge branch 'master' into feature/JENKINS-39370

ca68f83

[JENKINS-18578] - If workspace manager is defined, use JAR Cache with…

85384eb

…in its interbal directory

[JENKINS-39370] - Fix the workDirManager's log initialization in Laun…

ddd23f3

…cher

[JENKINS-39370] - Draft the documentation

0e73eeb

[JENKINS-39369] - Make JUL logging system configurable via property file

d7ef457

[JENKINS-39369] - Fixes in logging management after the manual testing

151c93e

oleg-nenashev added needs-tests and removed work-in-progress labels May 3, 2017

jglick requested review from stephenc and rsandell May 3, 2017 15:43

oleg-nenashev closed this May 3, 2017

oleg-nenashev reopened this May 3, 2017

oleg-nenashev added 2 commits May 4, 2017 14:32

[JENKINS-39369] - Add tests for the logging subsystem

21d8c80

[JENKINS-39369] - Respect configuration being passed from java.util.l…

658095b

…ogging.config.file system property

oleg-nenashev added needs-review and removed needs-tests labels May 4, 2017

recampbell requested a review from jglick May 5, 2017 15:48

stephenc approved these changes May 5, 2017

View reviewed changes

oleg-nenashev merged commit 76c9b8c into jenkinsci:master May 7, 2017

oleg-nenashev deleted the feature/JENKINS-39370 branch May 7, 2017 07:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-39370,JENKINS-39369] - Support of work directories in Remoting #129

[JENKINS-39370,JENKINS-39369] - Support of work directories in Remoting #129

oleg-nenashev commented Oct 31, 2016 •

edited

Loading

batmat Nov 2, 2016

oleg-nenashev Nov 2, 2016

batmat Nov 2, 2016

oleg-nenashev Nov 2, 2016

batmat Nov 2, 2016

oleg-nenashev Nov 2, 2016

oleg-nenashev Nov 16, 2016

oleg-nenashev commented Nov 17, 2016

oleg-nenashev commented Nov 27, 2016

stephenc left a comment

stephenc Dec 9, 2016

oleg-nenashev Dec 9, 2016

stephenc Dec 9, 2016

oleg-nenashev Dec 9, 2016

rsandell Dec 13, 2016

rsandell Dec 13, 2016

oleg-nenashev commented Feb 17, 2017

jglick commented Feb 17, 2017

oleg-nenashev commented May 3, 2017 •

edited

Loading

ghost commented May 3, 2017

oleg-nenashev commented May 4, 2017

stephenc left a comment

oleg-nenashev commented May 7, 2017

[JENKINS-39370,JENKINS-39369] - Support of work directories in Remoting #129

[JENKINS-39370,JENKINS-39369] - Support of work directories in Remoting #129

Conversation

oleg-nenashev commented Oct 31, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleg-nenashev commented Nov 17, 2016

oleg-nenashev commented Nov 27, 2016

stephenc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleg-nenashev commented Feb 17, 2017

jglick commented Feb 17, 2017

oleg-nenashev commented May 3, 2017 • edited Loading

ghost commented May 3, 2017

oleg-nenashev commented May 4, 2017

stephenc left a comment

Choose a reason for hiding this comment

oleg-nenashev commented May 7, 2017

oleg-nenashev commented Oct 31, 2016 •

edited

Loading

oleg-nenashev commented May 3, 2017 •

edited

Loading