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

Adjust installation test #100

Merged
merged 2 commits into from
Sep 7, 2021
Merged

Conversation

jkakavas
Copy link
Member

@jkakavas jkakavas commented Sep 5, 2021

During installation, we start elasticsearch a couple of times and
check that we can connect to it. Since elastic/elasticsearch#72300
these tests ( and installation for that matter) have been failing
since security is enabled by default and we make requests without
credentials.

This change, as a temporary measure, makes it so that we start
elasticsearch with security explicitly disabled temporarily during
installation so that we can make the necessary connection test
requests without credentials.

In a follow-up, we will adjust the Formula to take advantage of
newly added functionality, set and show the password for the
elastic user during installation (similar to what we do in
elastic/elasticsearch#75144). We can then also use this password
to make the test requests without needing to disable authN.

During installation, we start elasticsearch a couple of times and
check that we can connect to it. Since elastic/elasticsearch#72300
this tests ( and installation for that matter) has been failing
since security is enabled by default and we make requests without
credentials.

This change, as a temporary measure, makes it so that we start
elasticsearch with security explicitly disabled temporarily during
installation so that we can make the necessary connection test
requests without credentials.

In a follow-up, we will adjust the Formula to take advantage of
newly added functionality, set and show the password for the
elastic user during installation (similar to what we do in
elastic/elasticsearch#75144). We can then also use this password
to make the test requests without needing to disable authN.
@jkakavas jkakavas requested a review from mgreau September 5, 2021 19:06
Copy link

@breskeby breskeby left a comment

Choose a reason for hiding this comment

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

lgtm

@jkakavas
Copy link
Member Author

jkakavas commented Sep 7, 2021

@mgreau just making sure this is on your radar so that we can merge soon and unblock tests

Copy link
Member

@mgreau mgreau left a comment

Choose a reason for hiding this comment

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

LGTM

@mgreau mgreau merged commit 56b7b0b into elastic:master Sep 7, 2021
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