Upgrade to track 2 SDK, v6 => v12 #15
Conversation
| listener.getLogger().println(Messages.AzureArtifactManager_copy_all_fail(e)); | ||
| throw new IOException(e); | ||
| } | ||
| } | ||
|
|
||
| private int copyBlobs(Iterable<ListBlobItem> sourceBlobs, String toKey, CloudBlobContainer container) throws | ||
| StorageException, URISyntaxException { | ||
| private int copyBlobs(PagedIterable<BlobItem> sourceBlobs, String toKey, BlobContainerClient container) { |
There was a problem hiding this comment.
@jglick do you know what this is used for? / how to test it?
It's part of this API:
https://javadoc.jenkins.io/plugin/workflow-api/index.html?org/jenkinsci/plugins/workflow/flow/StashManager.StashAwareArtifactManager.html
There's no test coverage for it here, and I haven't managed to trigger this usecase.
Can't see it in the JEP either
There was a problem hiding this comment.
https://javadoc.jenkins.io/plugin/workflow-api/org/jenkinsci/plugins/workflow/flow/StashManager.StashAwareArtifactManager.html#copyAllArtifactsAndStashes-hudson.model.Run-hudson.model.TaskListener- you mean? Called from https://github.com/jenkinsci/workflow-api-plugin/blob/fa8a6b3e6d5248e4489545ccb33f1f623ad4c9bb/src/main/java/org/jenkinsci/plugins/workflow/flow/StashManager.java#L361 which would in turn run when you reran a Declarative build from a stage via https://github.com/jenkinsci/pipeline-model-definition-plugin/blob/15978cd172a8d34e2cd6bf8a6bfeaa5514d44dad/pipeline-model-definition/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/actions/RestartFlowFactoryAction.java#L71-L73. (Also a Scripted build from checkpoint if using CloudBees CI but you can ignore that for this purpose.) Looks like this gets test coverage via https://github.com/jenkinsci/workflow-api-plugin/blob/fa8a6b3e6d5248e4489545ccb33f1f623ad4c9bb/src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java#L277-L287 (currently not doing it more realistically via Declarative restart), so what you need to do is make sure you are using that test fixture from this plugin. https://github.com/jenkinsci/artifact-manager-s3-plugin/blob/39a52d2b6f8c75bb2bece55bcbc3a99a320f8332/src/test/java/io/jenkins/plugins/artifact_manager_jclouds/s3/MinioIntegrationTest.java#L188 is an example.
There was a problem hiding this comment.
❤️ this conformance test, mostly got it working but it picked up deeply nested dir not working for some reason, although one level of dir working
There was a problem hiding this comment.
ugh this requires re-writing of the archive method to make the test pass.
The test is failing because the .git directory is not being archived, .git isn't being archived because the storage plugin is using https://github.com/jenkinsci/azure-storage-plugin/blob/master/src/main/java/com/microsoftopentechnologies/windowsazurestorage/service/UploadService.java#L502
Which calls https://javadoc.jenkins.io/hudson/FilePath.html#list-java.lang.String-java.lang.String-
Which uses the default exclude list from ant, which excludes .git.
Also not sure how reasonable this test is, who is archiving .git directories =/
There was a problem hiding this comment.
Is this test correct?
The stashes are cleared here:
https://github.com/jenkinsci/workflow-api-plugin/blob/fa8a6b3e6d5248e4489545ccb33f1f623ad4c9bb/src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java#L288-L289
But the test seems to expect that the stash isn't deleted?
I think the s3 plugin passes because it defaults to ignoring deleting of stashes?
Is this part of the API? Or an implementation detail?
https://github.com/jenkinsci/artifact-manager-s3-plugin/blob/39a52d2b6f8c75bb2bece55bcbc3a99a320f8332/src/main/java/io/jenkins/plugins/artifact_manager_jclouds/JCloudsArtifactManager.java#L330-L333
Doesn't appear to be documented in javadoc or JEP
There was a problem hiding this comment.
The test is failing because the .git directory is not being archived
Sorry, what .git/ directory? Which test would be trying to archive that?
There was a problem hiding this comment.
I think the s3 plugin passes because it defaults to ignoring deleting of stashes?
It depends on which tests you run. Use the *AndDelete versions if your artifact manager supports deleting blobs, the base versions if it does not, or both if this behavior is configurable.
There was a problem hiding this comment.
For context, artifact-manager-s3 normally does not delete blobs because
- It is safer to not have to grant Jenkins that permission, which could be used to erase evidence. In the default mode you need only grant it permission to create new blobs.
- There are more efficient and reliable ways of deleting blobs in bulk according to some policy, or even moving them to archival storage like Glacier.
There was a problem hiding this comment.
The test is failing because the .git directory is not being archived
Sorry, what
.git/directory? Which test would be trying to archive that?
This test asserts that you can copy git directories, I solved that bit by rewriting the archive method
I think the s3 plugin passes because it defaults to ignoring deleting of stashes?
It depends on which tests you run. Use the
*AndDeleteversions if your artifact manager supports deleting blobs, the base versions if it does not, or both if this behavior is configurable.
Ah I see, yes this one does support it
For context, artifact-manager-s3 normally does not delete blobs because
It is safer to not have to grant Jenkins that permission, which could be used to erase evidence. > In the default mode you need only grant it permission to create new blobs.
I see, in the current setup of this / other plugins in the Azure eco-system it's not possible to prevent Jenkins having that permission. Azure has moved on and now has the required features to make that possible but it requires some plugin(s) factoring. I'll leave as is for now.
Thanks! The test now passes
There was a problem hiding this comment.
This test asserts that you can copy git directories
Ah sorry I had forgotten about https://github.com/jenkinsci/workflow-api-plugin/blob/fa8a6b3e6d5248e4489545ccb33f1f623ad4c9bb/src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java#L209 and https://github.com/jenkinsci/workflow-api-plugin/blob/fa8a6b3e6d5248e4489545ccb33f1f623ad4c9bb/src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java#L362 which I guess are trying to verify consistent behavior with the built-in artifact manager.
it's not possible to prevent Jenkins having that permission
Fine, that is just a design decision for this plugin, so you pick whichever test flavor is more appropriate.
Can't stream it unfortunately as each file has a different signed url
|
fyi @xuzhang3 if you have time |
need to figure out
copyBlobsmethod