Dedicated OSGi bundle merging both client-java and client-java-api

[azzazzel]

This is simple workaround for #737
It adds new module that basically merges the two modules currently containing the split package.

This is not a fix but workaround that allows to use the client in OSGi environment wile waiting for the proper fix in 7.

I'm sending this to master only because I couldn't find a 6.0.1 branch. It would be awesome if you can merge it to 6.0.1 code (only new files here so there should be no conflicts) and release the io.kubernetes:client-java-osgi:6.0.1 artifact to maven central.

[k8s-ci-robot]

Welcome @azzazzel!

It looks like this is your first PR to kubernetes-client/java 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-client/java has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[brendandburns]

This looks fine to me, thanks!

We have a flaky test that we need to fix, then we can merge on green...

[brendandburns]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: azzazzel, brendandburns

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [brendandburns]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[brendandburns]

@azzazzel this is still failing continuous testing for some reason. We've gotten rid of the flake (other PRs are passing at least) so I'm not sure what's going on here... Do tests pass locally for you?

[yue9944882]

[ERROR] /home/travis/build/kubernetes-client/java/client-java-osgi/bnd.bnd [0:0]: Can not find JAR file 'client-java-api-7.0.0-SNAPSHOT.jar'
[ERROR] /home/travis/build/kubernetes-client/java/client-java-osgi/bnd.bnd [0:0]: Can not find JAR file 'client-java-7.0.0-SNAPSHOT.jar'

it's not the flakiness in this pull i believe

[azzazzel]

Well, as this is a new artifact that only packages together binary files (classes) I didn't expect it to be involved in any tests. After all it operates on the result of already tested code. I can have another look at it this weekend but I'm afraid I'll need some more insides about your test expectations.

[azzazzel]

I'm responding from my phone and I can't see all the details but from the pasted snippet it seams that the problem is not in tests but that the travis build fails. Is that correct? Or are there any generic tests that are automatically applied to new artifacts during the CI pipeline?

[brendandburns]

You should be able to just run mvn test and it will run all of the tests.

If that runs clean on your local workspace then there's a problem in travis. If that doesn't run clean on your machine then we need a change to the PR.

Happy to help more if you have questions and thanks in advance for working on this!

[k8s-ci-robot]

New changes are detected. LGTM label has been removed.

[azzazzel]

When I run mvn test on the PR branch I get

[INFO]
[ERROR] Errors:
[ERROR]   SharedInformerFactoryTest.shutdownInformerFactoryInstantlyAfterStarting:13 »  ...
[ERROR]   DefaultSharedIndexInformerTest.testAllNamespacedPodInformerNormalBehavior:176 »
[ERROR]   DefaultSharedIndexInformerTest.testInformerReListWatchOnWatchConflict:261 »  U...
[ERROR]   DefaultSharedIndexInformerTest.testInformerReListingOnListForbidden:322 »  Unr...
[ERROR]   DefaultSharedIndexInformerTest.testNamespacedPodInformerNormalBehavior:80 »  U...
[INFO]
[ERROR] Tests run: 96, Failures: 0, Errors: 5, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] Kubernetes Client API .............................. SUCCESS [  0.002 s]
[INFO] client-java-api .................................... SUCCESS [ 45.952 s]
[INFO] client-java-proto .................................. SUCCESS [  0.087 s]
[INFO] client-java ........................................ FAILURE [02:17 min]
[INFO] client-java-extended ............................... SKIPPED
[INFO] client-java-examples ............................... SKIPPED
[INFO] client-java-osgi ................................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------

As you see the tests fail on client-java project which this PR does not touch. So I also run the tests on master commit 874e248 (the point at which I branched out) and I got the same test failure.

It looks like I started from a master where tests don't pass. So I pulled the latest master and run the tests and they all passed. Then I rebased my branch on the latest master and run the tests again and I got

[ERROR] /data/projects/3rdParty/kubernetes-client-java/client-java-osgi/bnd.bnd [0:0]: Can not find JAR file 'client-java-api-7.0.0-SNAPSHOT.jar'
[ERROR] /data/projects/3rdParty/kubernetes-client-java/client-java-osgi/bnd.bnd [0:0]: Can not find JAR file 'client-java-7.0.0-SNAPSHOT.jar'
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] Kubernetes Client API .............................. SUCCESS [  0.002 s]
[INFO] client-java-api .................................... SUCCESS [ 44.442 s]
[INFO] client-java-proto .................................. SUCCESS [  0.043 s]
[INFO] client-java ........................................ SUCCESS [01:19 min]
[INFO] client-java-extended ............................... SUCCESS [ 53.458 s]
[INFO] client-java-examples ............................... SUCCESS [  4.129 s]
[INFO] client-java-osgi ................................... FAILURE [  5.603 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------

My guess is that your concern is the second failure and not the first one. If so, it is not the tests failing. It happens because the bnd's includeresource instruction relies on the fact that the jars from the other two modules are available. When running mvn package those are generated in the reactor and available and there are not errors. But when running just mvn test the package phase is not executed on the other modules and the jars are not in the reactor which result in the above error.

To "fix" this issue and ensure it does not brake the CI, I simply moved the plugin execution to prepare-package phase. But please note that IMHO it makes no sense to merge this PR to master!

Please add this fix to 6.0.x

As far as I know you are already solving the split package problem for master (in #709 perhaps) the proper way. This PR was sent hoping to get it merged to 6.0.1 and get the new client-java-osgi artifact version 6.0.1 released to allow working around the issue in the current major version. If that does not work for some reason you can perhaps release 6.0.2 with the new artifact.

I can rebase and resend this PR to release-6.0 branch but I'm not sure that is the proper one. If it is, please let me know and will resend there. If not, I hope you have some ways to find the codebase for 6.0.1, copy these 3 files to it and release the extra jar to maven central.

Assuming you fix the issue the proper way in 7.0.0 you will no longer need the client-java-osgi module. Therefore merging the code to master does not make sense IMHO.

[brendandburns]

Ok, thanks for the info. The release-6.0 branch is the right branch to send this PR for. We can then merge and build a 6.0.2 with the fix.

Thanks

[azzazzel]

OK. Thanks for clarification. I looked at release-6.0 branch first but the POM there says 7.0.0-SNAPSHOT. So I thought my assumption was wrong and the branch was for something else.

I am closing this now and will resend on release-6.0 branch.