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.

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

Add ability to install feature packs on top of existing servers #426

Merged
merged 16 commits into from
Sep 7, 2023

Conversation

spyrkob
Copy link
Collaborator

@spyrkob spyrkob commented Aug 1, 2023

Support installing features pack on top of already provisioned servers.

Adds prospero features add operation, history and revert support

Comment on lines 99 to 106
// verify the datasources feature pack was installed successfully
final Path modulePath = Path.of("modules", "com", "mysql", "jdbc");
assertThat(targetDir.toPath().resolve(modulePath))
.exists()
.isDirectory();
Copy link
Contributor

@TomasHofman TomasHofman Aug 2, 2023

Choose a reason for hiding this comment

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

Just a suggestion - in this test we could check that originally the feature pack is missing, before running the add command.

@spyrkob spyrkob changed the title Add features Add ability to install feature packs on top of existing servers Aug 2, 2023
public Integer call() throws Exception {
final long startTime = System.currentTimeMillis();

if (fpl.split(":").length != 2) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The version part is not valid input?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, the version of feature pack is a part of the channel

private final InstallationMetadata metadata;
private final ProsperoConfig prosperoConfig;
private final Console console;
private CandidateActionsFactory candidateActionsFactory;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be final too.

@@ -50,20 +48,19 @@
class PrepareCandidateAction implements AutoCloseable{
Copy link
Contributor

@TomasHofman TomasHofman Aug 2, 2023

Choose a reason for hiding this comment

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

I'm not sure what to imagine under "candidate" term here. Is that just a working installation that maybe gets used at the end of the process?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. The candidate is basically a newly provisioned server with the new feature pack. In next step that candidate is merged with existing server to apply the changes. This is a process that happened in Galleon under the hood, but we explicitly split it up in prospero to allow preparing candidate without stopping the server (in case of update/revert at least).

@TomasHofman
Copy link
Contributor

TomasHofman commented Aug 3, 2023

I'm thinking the .provisioning_record.xml should not have the leading dot? It's already in a directory with leading dot, there's no point in hiding it again... It's also inconsistent considering the remaining files in .installation dir.

[thofman@fedora prospero]$ ls eap80/.installation/ -a
.   .cache  installer-channels.yaml  manifest.yaml             README.txt
..  .git    manifest_version.yaml    .provisioning_record.xml

@TomasHofman
Copy link
Contributor

That may be out of scope for this PR, but it feels to me that I'm missing some command to review the current state of the installation. I can add a new feature pack but I can't review currently installed feature packs. Something like features list. Then probably also remove.

Maybe I would like to call the command feature-pack and create a shortcut name fp - just thinking about consistent terminology with the maven plugin and docs...

What about some general status command that would display all the important data about the installation? (channel URL or channel version or something, feature packs, model, configs, etc)

@TomasHofman
Copy link
Contributor

And a little brainstorming - we could consider something for user to discover available feature packs / models / layers.

@TomasHofman
Copy link
Contributor

I think we should also add some paragraph in the docs about this. Correct procedure to follow, since it can require multiple commands to install e.g. the datasource and db driver.

@TomasHofman
Copy link
Contributor

TomasHofman commented Aug 3, 2023

BTW I'm trying following:

./prospero install --profile=eap-8.0 --channel=https://.../jboss-set/channel-definitions/-/raw/main/eap-80-test-build/channel.yaml --dir eap80
./prospero features add --dir eap80/ --fpl org.jboss.eap:eap-datasources-galleon-pack --layers datasources-web-server,postgresql-datasource

without creating additional channel and providing driver version. It results in a NPE:

...
Feature-packs resolved.                                                          
Packages installed.               
Downloading artifacts 1/593(0%) activemq-artemis-native-1.0.2.redhat-00003.jarjava.lang.NullPointerException
	at java.base/java.util.Objects.requireNonNull(Objects.java:209)
	at org.wildfly.channel.ArtifactCoordinate.<init>(ArtifactCoordinate.java:33)
	at org.wildfly.prospero.galleon.MavenArtifactMapper.lambda$toChannelArtifacts$0(MavenArtifactMapper.java:59)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
	at org.wildfly.prospero.galleon.MavenArtifactMapper.toChannelArtifacts(MavenArtifactMapper.java:60)
	at org.wildfly.prospero.galleon.ChannelMavenArtifactRepositoryManager.resolveAll(ChannelMavenArtifactRepositoryManager.java:188)
	at org.wildfly.galleon.plugin.WfInstallPlugin.resolveArtifactsInCache(WfInstallPlugin.java:555)
	at org.wildfly.galleon.plugin.WfInstallPlugin.postInstall(WfInstallPlugin.java:410)
	at org.jboss.galleon.runtime.ProvisioningRuntime$3.visitPlugin(ProvisioningRuntime.java:269)
	at org.jboss.galleon.runtime.ProvisioningRuntime$3.visitPlugin(ProvisioningRuntime.java:266)
	at org.jboss.galleon.layout.ProvisioningLayout$Handle.visitPlugins(ProvisioningLayout.java:287)
	at org.jboss.galleon.layout.ProvisioningLayout.visitPlugins(ProvisioningLayout.java:860)
	at org.jboss.galleon.runtime.ProvisioningRuntime.provision(ProvisioningRuntime.java:266)

Update:

I don't have this issue when working with the wildfly-27.0.1 manifest from the intergration-tests dir and the wildfly galleon pack.

@spyrkob
Copy link
Collaborator Author

spyrkob commented Aug 3, 2023

That may be out of scope for this PR, but it feels to me that I'm missing some command to review the current state of the installation. I can add a new feature pack but I can't review currently installed feature packs. Something like features list. Then probably also remove.

That's good idea. I think the status operation would be a good addition for this PR, remove is definately out of scope of this RFE, but I'll create a issue to track it.

Maybe I would like to call the command feature-pack and create a shortcut name fp - just thinking about consistent terminology with the maven plugin and docs...

I like that - @honza-kasik, I think that would affect your tests WDYT?

And a little brainstorming - we could consider something for user to discover available feature packs / models / layers.

Yes there's already #428 to discover available feature packs, and I added #429 to list models and layers. That's something we can add later on though.

I think we should also add some paragraph in the docs about this. Correct procedure to follow, since it can require multiple commands to install e.g. the datasource and db driver.

Yes, absolutely. I need to finish #153 and add it there

@spyrkob
Copy link
Collaborator Author

spyrkob commented Aug 8, 2023

@TomasHofman I added a docs entry. could you take a quick look when you have a moment?

Comment on lines +17 to +19
- groupId: org.wildfly
artifactId: wildfly-datasources-galleon-pack
version: 4.0.1.Final
Copy link
Contributor

Choose a reason for hiding this comment

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

Do I remember correctly the datasources galleon pack is going to be part of the main channel?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

AFAIK, not in the community version.

dist/docs/guide/usage/add_feature_pack.adoc Show resolved Hide resolved
@spyrkob spyrkob force-pushed the add_features branch 2 times, most recently from 4e1b51c to 90e72fb Compare August 10, 2023 11:47

#### Offline installation

Similarly to other operations, `feature-pack add` can be executed in an offline mode. To do so, users should provide local repositories with all required artifacts (both for base server and the new feature) using `--repositories` parameter.

Choose a reason for hiding this comment

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

I'm missing description of --repositories in the feature analysis. Can user also define a channel (with a manifest) for a new feature pack and then use --repositories=<offline-feature-pack-channel-repo>,<offline-base-channel-repo>?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@honza-kasik yes the --repositories work in the same way as in update operation so the user would have to provide both base and feature pack repositories

@spyrkob spyrkob force-pushed the add_features branch 2 times, most recently from 9ded83d to 70a3e7f Compare September 4, 2023 08:54
@spyrkob spyrkob merged commit 48545a9 into wildfly-extras:main Sep 7, 2023
5 checks passed
@spyrkob spyrkob deleted the add_features branch September 7, 2023 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants