Skip to content
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

Jump to bottom

Fix #106: ISM Plugin is not functional #133

Merged
merged 2 commits into from
Apr 11, 2017
Merged

Fix #106: ISM Plugin is not functional #133

merged 2 commits into from
Apr 11, 2017

Conversation

neilxb
Copy link
Contributor

@neilxb neilxb commented Apr 10, 2017

NB this PR does not directly fix the ISM issue #106 , which no longer occurs after the merge of PR #111. However after that merge there was a resolution error when attempting to install the sample plugin due to a conflict with the bundles in the platform. This PR fixes that resolution error and also adds additional diagnostic info in cases where a resolution error occurs.

@emanoelxavier
Copy link
Contributor

@neilxb can you provide more details on what was the resolution error (i.e.: what caused it and how it is no longer happening)? Also given that the ISM issue was fixed on #111 should #106 be closed?

@neilxb neilxb mentioned this pull request Apr 11, 2017
Closed
arvindn05
arvindn05 approved these changes Apr 11, 2017
View reviewed changes
osc-installer/src/main/java/org/osc/core/server/installer/impl/DeploymentInstaller.java Outdated
@@ -278,6 +279,7 @@ void deactivate() {
oldState = unit.getState();
unit.setState(State.ERROR);
unit.setErrorMessage(e.getMessage());
if (log != null) log.log(LogService.LOG_ERROR, "Error installing artifact(s)", e);
Copy link
Member

Choose a reason for hiding this comment

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

checkstyle issue

@neilxb
Copy link
Contributor Author

neilxb commented Apr 11, 2017

The resolution error that occurred was as follows. Both the SampleMgrPlugin.bar and the application itself contain the bundle tx-control-service-local-0.0.2.jar. In certain cases the resolver can produce a solution which contains a bundle that is already present in the base framework -- this happens because OSGi resolving is expensive and the resolver does not exhaustively search for the best possible solution but stops when it find a solution.

When this occurs, the FrameworkInstallerComponent throws an exception upon install of the bundle because OSGi does not normally allow multiple bundles to exist with the same Bundle-SymbolicName/Bundle-Version pair. I expect this to be rare. In the PR we catch that specific error from OSGi and downgrade it to a warning.

Note that in this case the duplicate bundle does not form part of the reference-counted installation set for the plugin .bar file, and therefore will not be uninstalled when the plugin is removed. I think this is the correct behaviour.

@neilxb
Copy link
Contributor Author

neilxb commented Apr 11, 2017

Thanks for the approval.

@neilxb neilxb merged commit 6b4d2fb into master Apr 11, 2017
@neilxb neilxb deleted the nbartlex/issue106 branch April 11, 2017 16:12
@emanoelxavier
Copy link
Contributor

emanoelxavier commented Apr 11, 2017 •
edited
Loading

@neilxb looks like this PR still includes 5 warnings http://10.3.240.52:8080/view/Master/job/osc-core/170/ can you send a new PR fixing the warnings? thx!

emanoelxavier pushed a commit that referenced this pull request Apr 13, 2017
@neilxb
Fix #106: ISM Plugin is not functional (#133)
bfbc66a 
* Warn but don't fail on attempt to install duplicate bundle

* Address style issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arvindn05 arvindn05 arvindn05 approved these changes

@emanoelxavier emanoelxavier emanoelxavier approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@neilxb @emanoelxavier @arvindn05