Skip to content

JEP-202 updates, esp.: remove FilePath.copyFromRemotely from API #117

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.

Sign up for GitHub

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

Merged
bitwiseman merged 2 commits into from
Jul 3, 2018
Merged

JEP-202 updates, esp.: remove FilePath.copyFromRemotely from API #117

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0616b9c
Remove FilePath.copyFromRemotely from the API
jglick Jun 5, 2018
54c4f05
More updates to reflect reality.
jglick Jun 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions jep/202/README.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,6 @@ This change adds more methods to the http://javadoc.jenkins.io/jenkins/util/Virt
* `readLink()` If this file is a symlink, returns the link target.
* `list(String, String, boolean)` Lists recursive files of this directory with pattern matching

http://javadoc.jenkins.io/hudson/FilePath.html[`FilePath`]

* `copyFromRemotely(URL)` Copies the content of a URL to a remote file without transferring content over a Remoting channel


In https://github.com/jenkinsci/workflow-api-plugin[workflow-api-plugin] a new https://github.com/jglick/workflow-api-plugin/blob/44bc9985b0aa1fb9407b5c78be6b429ad29af2d8/src/main/java/org/jenkinsci/plugins/workflow/flow/StashManager.java#L292-L315[`StashAwareArtifactManager`] is also added, a mixin interface for an `ArtifactManager` which supports specialized stash behavior as well.

Expand All @@ -119,6 +115,8 @@ The `VirtualFile.run(Callable)` method, that already existed to optimize Workspa
It was important to implement this in the S3 plugin in order to ensure that Classic or Blue Ocean UI flows relating to artifacts, including simply opening the index page of a build, did not make a large number of network calls.
The cache system allows a single S3 metadata call (possibly one HTTP request, depending on page size) to retrieve sufficient information for a complete page render of some or all of the artifact tree, without the rendering code (e.g., in Jenkins core) knowing anything about the storage system.

The `apache-httpcomponents-client-4-api` plugin is extended with a `RobustHTTPClient`
Copy link
Member Author

@jglick jglick Jun 13, 2018

Choose a reason for hiding this comment

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

This has been released (thanks @dwnusbaum) but unfortunately (INFRA-1600) Javadoc is not published in an unpacked format.

Copy link
Member

@daniel-beck daniel-beck Jul 4, 2018

Choose a reason for hiding this comment

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

http://javadoc.jenkins.io/plugin/apache-httpcomponents-client-4-api/io/jenkins/plugins/httpclient/RobustHTTPClient.html

which standardizes handling of blob upload and download operations.

== Motivation

Expand All @@ -130,7 +128,7 @@ There are serious drawbacks to use filesystem when running in cloud or container
* Causing all sorts of issues due to usage of remoting for file transfer.
Going through the master as files are copied from/to the agent cause load, cpu, network issues that are hard to diagnose and recover from.
* Causing a big performance hit due to the traffic and disk access, preventing the usage of distributed filesystems such as NFS, GlusterFS,...
* Providing limited functionality compared to a first class blob store (AWS S3, Azure Blob Storage, Google Blobstore) or artifact repository (Nexus, Artifactory).
* Providing limited functionality compared to a first class blob store (AWS S3, Azure Blob Storage, Google Blobstore).
Copy link
Member Author

@jglick jglick Jun 13, 2018

Choose a reason for hiding this comment

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

@svanoort read the original version to imply that we would advocate using JEP-202 infrastructure to connect to artifact repositories in addition to blob stores, which is misleading. As noted in jenkinsci/workflow-api-plugin#67 (comment), Jenkins artifacts were never really “competing” with artifact repositories to begin with: people with a reason to use a (particular) artifactory repository should do so and ignore the simplistic notion of “artifact” built into Jenkins. Probably this should be expanded on in another section of the JEP as background.

Copy link
Member

@svanoort svanoort Jun 13, 2018

Choose a reason for hiding this comment

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

@jglick Sorry, but I'm afraid that's putting incorrect words in my mouth -- what I said is that they were "mentioned" originally in JEP-202 which is 100% true. I never said you were advocating for it, but that you seem to be making a completely arbitrary distinction for what can and cannot integrate with the Jenkins artifact mechanism as an external store. As far as I'm concerned the reason seems to be more implementation convenience than anything in the user experience.

I will remind you that you made similar arbitrary assumptions about what users would be doing with Pipeline files, and that they would be quite simple -- assumptions that have proven incorrect.

To me, if feels obvious to ask "okay, so you support S3 now, when can I just use Artifactory/Nexus/whatever and drop the custom steps to upload artifacts?" It lets people simplify their CI/CD cycle by making the actual artifact store a mere configuration detail.

Copy link
Member Author

@jglick jglick Jun 13, 2018

Choose a reason for hiding this comment

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

the reason seems to be more implementation convenience than anything in the user experience

No, the reason is very much user experience. If someone does ask

when can I just use Artifactory/Nexus/whatever and drop the custom steps to upload artifacts?

then the answer will be “never”. It would be a step backwards.

Organizations may have some requirements that are not available today: auditability, sophisticated retention policies, versioning,...

From the usability point of view, currently a Jenkins installation sizing has to have into account the expected amount of disk needed or a restart, disk reprovisioning, disk swap,... instead of a infinite scale, pay per use that a blob store can offer. Cloud installations can automatically configure the blob store to use offering a much better first time user experience.
Expand Down Expand Up @@ -166,16 +164,16 @@ The S3 implementation also uses http://jclouds.apache.org[Apache JClouds] that a

=== Container (Bucket) and Path References

Current implementation for S3 uses a master wide config option to set the name of the container (S3 bucket) and path inside.
Currently this is a environment variable but could be a system property or a config section in the UI.
The implementation for S3 uses a master-wide configuration option to set the name of the container (S3 bucket) and path inside.
(AWS-specific installers for Jenkins, such as for Essentials, could preconfigure these fields.)

This means that different runs cannot store the artifacts in different buckets or paths, as we don't expect that to be a common use case.
It would be more common to move all the artifacts from one location to another and that could be easily achieved by moving the blobs in S3 and changing the master wide configuration parameters.

=== Interruptions

The API does not support `InterruptedException`, but we do not see any evidence that it may cause integration issues with other plugins like Build Timeout.
Follow up work as part of https://issues.jenkins-ci.org/browse/JENKINS-50597[JENKINS-50597] will verify this.
The `VirtualFile` API does not support `InterruptedException`, but there is no evidence that it matters.
Test coverage confirms reasonable handling of error conditions including build timeouts and user aborts.

=== Security

Expand Down Expand Up @@ -294,7 +292,9 @@ Automated tests for the common archive/unarchive and stash/unstash flow have bee

The AWS S3 implementation tests exercise this flow plus add some extra S3 specific tests.
They require an AWS account and S3 permissions and can be run from a EC2 instance or a local machine.
Some mock testing can be added in the future.

There is an abstraction layer allowing use of any blob store supported by Apache jclouds.
This layer has its own mock tests confirming general behaviors.

== Prototype Implementation

Expand Down