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

Install all plugins during bats tests #13076

Merged
merged 1 commit into from Sep 1, 2015
Merged

Install all plugins during bats tests #13076

merged 1 commit into from Sep 1, 2015

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Aug 24, 2015

Related to #12717

@nik9000 nik9000 added >test Issues or PRs that are addressing/adding tests review :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts v2.1.0 labels Aug 24, 2015
@nik9000
Copy link
Member Author

nik9000 commented Aug 24, 2015

This only installs and removes that plugins using the plugin script - it doesn't try to run elasticsearch with them all installed. I'm not sure vagrant the vagrant tests are the appropriate place to do that since they are ostensibly repeated once per target OS.

@nik9000
Copy link
Member Author

nik9000 commented Aug 24, 2015

Ping @tlrx for more fun bats code to review. This one has a symlink!

nik9000
nik9000 reviewed Aug 24, 2015
View reviewed changes
qa/vagrant/src/test/resources/packaging/scripts/25_tar_plugins.bats
@@ -0,0 +1 @@
plugin_test_cases.bash
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a symlink but its hard to read in github's ui.

This was referenced Aug 24, 2015
Merged
Closed
@nik9000
Copy link
Member Author

nik9000 commented Aug 25, 2015

@brwe , this is one of the in flight bats changes.

@nik9000
Copy link
Member Author

nik9000 commented Aug 27, 2015

@spinscale, want to review some packaging tests?

@drewr
Copy link
Contributor

drewr commented Aug 31, 2015

Needed to use VAGRANT_DEFAULT_PROVIDER=virtualbox to get this to work for me. I suggest we explicitly set --provider=virtualbox wherever we call vagrant up.

@nik9000
Copy link
Member Author

nik9000 commented Aug 31, 2015

Needed to use VAGRANT_DEFAULT_PROVIDER=virtualbox to get this to work for me. I suggest we explicitly set --provider=virtualbox wherever we call vagrant up.

I thought that was the default. I can certainly do that though.

@dakrone
Copy link
Member

dakrone commented Aug 31, 2015

Needed to use VAGRANT_DEFAULT_PROVIDER=virtualbox to get this to work for me.

Same here, otherwise vagrant tries to use libvirt or something.

@nik9000
Copy link
Member Author

nik9000 commented Aug 31, 2015

Same here, otherwise vagrant tries to use libvirt or something.

wat? The default for vagrant for forever was virtualbox. Anyway, #13217.

@drewr
Copy link
Contributor

drewr commented Aug 31, 2015

@nik9000 It should be the default, but somehow because I use lxc in other contexts it seems to be sticking around somewhere. I even ran with env - mvn .... and it found it. Maybe just Vagrant 💩.

@drewr
Copy link
Contributor

drewr commented Aug 31, 2015

env VAGRANT_DEFAULT_PROVIDER=virtualbox mvn -Dtests.vagrant clean verify built successfully :bowtie:

@nik9000 nik9000 added the v2.0.0 label Aug 31, 2015
@nik9000
Copy link
Member Author

nik9000 commented Aug 31, 2015

I've added the 2.0.0 label to this to make it clear that this is going to the 2.0 branch. I'll need to merge #13223 before I can merge this to the 2.0 branch but I'll get it before closing this pr.

@nik9000
Copy link
Member Author

nik9000 commented Sep 1, 2015

@tlrx, would you mind reviewing this on its own? I think it might be ok and its worth getting in if we can.

tlrx
tlrx reviewed Sep 1, 2015
View reviewed changes
qa/vagrant/src/test/resources/packaging/scripts/plugin_test_cases.bash
#!/usr/bin/env bats

# This file is used to test the installation and removal
# of plugins with a tar gz archive.
Copy link
Member

Choose a reason for hiding this comment

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

Not only tar gz archive now

@nik9000
Copy link
Member Author

nik9000 commented Sep 1, 2015

@tlrx, would you mind reviewing this on its own? I think it might be ok and its worth getting in if we can.

Thanks! I'll read these and fix.

tlrx
tlrx reviewed Sep 1, 2015
View reviewed changes
qa/vagrant/src/test/resources/packaging/scripts/plugin_test_cases.bash
CONF_DIR="$ESCONFIG" remove_jvm_example
}

@test "[$GROUP] install jvm-example plugin with a custom ES_JAVA_OPTS" {
Copy link
Member

Choose a reason for hiding this comment

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

There's good chance that this test becomes obsolete soon with #12801... I'm +1 for removing it now

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like it just won't work with that in there. I can drop the test.

Copy link
Member

Choose a reason for hiding this comment

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

++

@tlrx
Copy link
Member

tlrx commented Sep 1, 2015

I like these changes, great work @nik9000 ! I read carefully the bats files and I made some minor comments.

Tests succeed on my laptop for precise, trusty and centos-7 (still have to remove the elasticsearch-rpm dependency to get them work). Please let me know if you want me to test a specific VM.

The last and most important thing I see is that for now the test installs a plugin, check for files and remove the plugin. That would be nice to start elasticsearch with the installed plugin and checks that it started correctly and that the plugin is referenced in Nodes Info API... What do you think?

@nik9000
Copy link
Member Author

nik9000 commented Sep 1, 2015

Tests succeed on my laptop for precise, trusty and centos-7 (still have to remove the elasticsearch-rpm dependency to get them work). Please let me know if you want me to test a specific VM.

Those vms are fine.

The last and most important thing I see is that for now the test installs a plugin, check for files and remove the plugin. That would be nice to start elasticsearch with the installed plugin and checks that it started correctly and that the plugin is referenced in Nodes Info API... What do you think?

Yeah - we probably should do it. I think that should be in another pr though.

@tlrx
Copy link
Member

tlrx commented Sep 1, 2015

Yeah - we probably should do it. I think that should be in another pr though.

Sounds good. Can you create an issue please?

@tlrx
Copy link
Member

tlrx commented Sep 1, 2015

LGTM then

@nik9000 nik9000 mentioned this pull request Sep 1, 2015
Closed
@nik9000
Copy link
Member Author

nik9000 commented Sep 1, 2015

Yeah - we probably should do it. I think that should be in another pr though.

#13254

LGTM then

In that case I'll squash, rebase, and merge.

nik9000 added a commit that referenced this pull request Sep 1, 2015
@nik9000
Merge pull request #13076 from nik9000/bats_plugins
69b7fee 
Packaging: Install all plugins during bats tests
@nik9000 nik9000 merged commit 69b7fee into elastic:master Sep 1, 2015
@nik9000
Copy link
Member Author

nik9000 commented Sep 1, 2015

Merged to master. Tried to merge it to 2.0 and found some unbackported stuff:
391ea37
0b650ed

Trying to backport those before merging this into 2.0.

@nik9000 nik9000 mentioned this pull request Sep 1, 2015
Merged
@nik9000 nik9000 changed the title Packaging: Install all plugins during bats tests Install all plugins during bats tests Sep 1, 2015
@nik9000
Copy link
Member Author

nik9000 commented Sep 2, 2015

Trying to backport those before merging this into 2.0.

Did that in #13255.

@nik9000
Copy link
Member Author

nik9000 commented Sep 2, 2015

And merged to 2.0! Yay! All done.

@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts Team:Delivery Meta label for Delivery team >test Issues or PRs that are addressing/adding tests v2.0.0-beta2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@nik9000 @drewr @dakrone @tlrx @clintongormley @mark-vieira