Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

Add war generator function #41

Merged
merged 13 commits into from
Jun 23, 2020

Conversation

sladyn98
Copy link
Contributor

@sladyn98 sladyn98 commented Jun 8, 2020

This PR adds the war generator function using the custom war packager library. It pulls in the packager config from the resources folder.

The Jenkinsfile Needs to be fixed before this can be merged in
Closes #14

@sladyn98 sladyn98 added enhancement New feature or request backend Related to the backend service labels Jun 8, 2020
@sladyn98 sladyn98 added this to the [GSoC Phase 1] milestone Jun 8, 2020
@sladyn98 sladyn98 requested a review from a team as a code owner June 8, 2020 13:39
@sladyn98 sladyn98 self-assigned this Jun 8, 2020
Copy link
Contributor

@kwhetstone kwhetstone left a comment

Choose a reason for hiding this comment

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

I can see this working. Eventually there probably should be a way to clean out /tmp

@sladyn98
Copy link
Contributor Author

sladyn98 commented Jun 9, 2020

I can see this working. Eventually there probably should be a way to clean out /tmp

@kwhetstone Should we do it after the user has downloaded it, But one minor inconvenience that would cause is that if the user hits download multiple times then we wouldn't have it in temp and would have to regenerate it

@sladyn98 sladyn98 closed this Jun 9, 2020
@sladyn98 sladyn98 reopened this Jun 9, 2020
@sladyn98
Copy link
Contributor Author

sladyn98 commented Jun 9, 2020

@kwhetstone I guess we can merge this once the build passes :)

Edit : Failed to execute goal on project custom-distribution-service: Could not resolve dependencies for project io.jenkins.tools.distribution-service:custom-distribution-service:jar:0.0.1: Could not find artifact io.jenkins.tools.custom-war-packager:custom-war-packager-lib:jar:2.0-alpha-3-SNAPSHOT -> [Help 1]
CC @LinuxSuRen


@Test
public void testWarGenerationSucceeds() {
WarGenerator.generateWAR("1.0");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kwhetstone @LinuxSuRen When I try to test this the builder throws a huge error

java.io.IOException: Command failed with exit code 1: mvn clean package
	at io.jenkins.tools.warpackager.lib.util.SystemCommandHelper.processFor(SystemCommandHelper.java:39)
	at io.jenkins.tools.warpackager.lib.util.MavenHelper.run(MavenHelper.java:66)
	at io.jenkins.tools.warpackager.lib.util.MavenHelper.run(MavenHelper.java:46)
	at io.jenkins.tools.warpackager.lib.impl.Builder.build(Builder.java:146)
	at com.org.jenkins.custom.jenkins.distribution.service.generators.WarGenerator.generateWAR(WarGenerator.java:24)
	at com.org.jenkins.custom.jenkins.distribution.service.generators.WarGeneratorTest.testWarGenerationSucceeds(WarGeneratorTest.java:9)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open issue in Custom War Packager.

Copy link
Member

@LinuxSuRen LinuxSuRen left a comment

Choose a reason for hiding this comment

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

I'm not sure why you want to keep tmp/prebuild/pom.xml.

final File configPath = util.getFileFromResources(PACKAGER_CONFIG_YAML);
try {
cfg = Config.loadConfig(configPath);
cfg.buildSettings.setTmpDir(new File(DEFAULT_TMP_DIR_NAME));
Copy link
Member

Choose a reason for hiding this comment

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

You can get a temporary directory by the following code:
System.getProperty("java.io.tmpdir")
Furthermore, a user can give it a different value by passing a path. For example: java -Djava.io.tmpdir=/path/to/tmpdir

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes made the change 👍

Copy link
Member

@LinuxSuRen LinuxSuRen left a comment

Choose a reason for hiding this comment

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

LGTM

@sladyn98 sladyn98 requested a review from LinuxSuRen June 10, 2020 06:12
@sladyn98
Copy link
Contributor Author

sladyn98 commented Jun 10, 2020

@LinuxSuRen The test is throwing errors.
Edit 1: By using the temp directory given by the system, the war packager throws an error, because when it tries cleaning up the temporary directory this is what happens

ava.io.IOException: Directory /tmp/.font-unix unable to be deleted.
	at org.codehaus.plexus.util.FileUtils.deleteDirectory(FileUtils.java:1681)
	at org.codehaus.plexus.util.FileUtils.forceDelete(FileUtils.java:1464)
	at org.codehaus.plexus.util.FileUtils.cleanDirectory(FileUtils.java:1731)
	at org.codehaus.plexus.util.FileUtils.deleteDirectory(FileUtils.java:1677)
	at io.jenkins.tools.warpackager.lib.impl.Builder.build(Builder.java:78)

Have a look at the code here: https://github.com/jenkinsci/custom-war-packager/blob/3aa29ed5432051198b9de410223732cc21843f36/custom-war-packager-lib/src/main/java/io/jenkins/tools/warpackager/lib/impl/Builder.java#L78

@LinuxSuRen
Copy link
Member

It might be caused by some restriction OS environments.

@sladyn98
Copy link
Contributor Author

Maybe that is why it asks for a tmp directory within the project structure and not the system temp directory, should we just revert to using the project temp directory instead of the system one ?

@LinuxSuRen
Copy link
Member

LinuxSuRen commented Jun 10, 2020

Maybe that is why it asks for a tmp directory within the project structure and not the system temp directory, should we just revert to using the project temp directory instead of the system one ?

I think that you can pass it another directory. It is already configrable now.

@sladyn98
Copy link
Contributor Author

I think that you can pass it another directory. It is already configrable now.

Like what ? Because it would anyway be created in the project structure, so does it matter if I pass temp again ?

@LinuxSuRen
Copy link
Member

Or you can get the temporary directory by reading a config file.

@sladyn98
Copy link
Contributor Author

sladyn98 commented Jun 10, 2020

Or you can get the temporary directory by reading a config file.

I do not think it would make a difference since the value in the config file would be static. It is a web service so there would be no way to change it

@kwhetstone
Copy link
Contributor

If using Java you can write a temp file using Files. I think all you have to do here is call delete on the file object at the end of all this.

@kwhetstone
Copy link
Contributor

@sladyn98 let's update the Jenkinsfile and see if that fixes the problem.
lines

@kwhetstone
Copy link
Contributor

@sladyn98 Looks like there's an error while trying to build the job. org.springframework.beans.factory.BeanDefinitionStoreException: Failed to parse configuration class [com.org.jenkins.custom.jenkins.distribution.service.CustomJenkinsDistributionServiceApplication]; nested exception is org.springframework.context.annotation.ConflictingBeanDefinitionException: Annotation-specified bean name 'updateCenterService' for bean class [com.org.jenkins.custom.jenkins.distribution.service.services.UpdateCenterService] conflicts with existing, non-compatible bean definition of same name and class [com.org.jenkins.custom.jenkins.distribution.service.UpdateCenterService]

The reason the build failed was looking for test reports and it couldn't find it. You can configure archiveArtifacts allowEmptyArchive: true, artifacts: '**/target/**/*.jar

Copy link
Contributor

@kwhetstone kwhetstone left a comment

Choose a reason for hiding this comment

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

Awesome passing build! Let's get this merged in!

@sladyn98 sladyn98 merged commit ae885a7 into jenkinsci:master Jun 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backend Related to the backend service enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement generation of WAR using CWP libraries
3 participants