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

fix(quickstart): elasticsearch-setup script fails on curl #5975

Merged
merged 1 commit into from
Sep 19, 2022

Conversation

shirshanka
Copy link
Contributor

@shirshanka shirshanka commented Sep 18, 2022

PR #5887 caused a failure in the elasticsearch-setup script when insecure mode is not used, due to a sneaky bug.

curl -o /dev/null .. -s "$ELASTICSEARCH_INSECURE" "$ELASTICSEARCH_PROTOCOL://...." 

will result in:
curl -o /dev/null .. -s "" "http://localhost:9002/..." 

which results in curl trying to hit 2 urls... the first being an empty string. 

This PR fixes this by re-attaching the ELASTICSEARCH_INSECURE flag to the beginning of the url
"${ELASTICSEARCH_INSECURE}$ELASTICSEARCH_PROTOCOL://..."

a better fix would probably be to refactor the CURL_OPTS into a separate variable that adds the -k flag if needed.

Would like to get this PR in first to limit the refactor.

Tested locally by building the docker image and verifying that a clean quickstart now completes cleanly with es-indexes set up correctly.

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

@github-actions github-actions bot added the devops PR or Issue related to DataHub backend & deployment label Sep 18, 2022
POLICY_RESPONSE_CODE=$(curl -o /dev/null -s -w "%{http_code}" --header "$ELASTICSEARCH_AUTH_HEADER" "$ELASTICSEARCH_INSECURE" "$ELASTICSEARCH_PROTOCOL://$ELASTICSEARCH_HOST:$ELASTICSEARCH_PORT/_ilm/policy/${PREFIX}datahub_usage_event_policy")
echo -e "Create datahub_usage_event if needed against Elasticsearch at $ELASTICSEARCH_HOST:$ELASTICSEARCH_PORT"
echo -e "Going to use index prefix:$PREFIX:"
POLICY_RESPONSE_CODE=$(curl -o /dev/null -s -w "%{http_code}" --header "$ELASTICSEARCH_AUTH_HEADER" "${ELASTICSEARCH_INSECURE}$ELASTICSEARCH_PROTOCOL://$ELASTICSEARCH_HOST:$ELASTICSEARCH_PORT/_ilm/policy/${PREFIX}datahub_usage_event_policy")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just curious -- Why is all of the new __STATUS stuff required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

found the real problem... fixed up the PR

@shirshanka shirshanka merged commit 6f48356 into datahub-project:master Sep 19, 2022
tomas-kubin added a commit to tomas-kubin/datahub that referenced this pull request Sep 20, 2022
@WarFox
Copy link

WarFox commented Sep 27, 2022

2022/09/27 11:32:26 Waiting for: http://elasticsearch-master:9200
2022/09/27 11:32:26 Received 200 from http://elasticsearch-master:9200
Going to use protocol: http
Going to use default elastic headers
Create datahub_usage_event if needed against Elasticsearch at elasticsearch-master:9200
Going to use index prefix::
curl: option -k http://elasticsearch-master:9200/_ilm/policy/datahub_usage_event_policy: is unknown
curl: try 'curl --help' or 'curl --manual' for more information
Policy GET response code is
/create-indices.sh: line 41: [: -eq: unary operator expected
/create-indices.sh: line 45: [: -eq: unary operator expected
/create-indices.sh: line 47: [: -eq: unary operator expected
Got response code  while creating policy so exiting.
2022/09/27 11:32:26 Command exited with error: exit status 1

We're getting this failure when upgrading to linkedin/datahub-elasticsearch-setup:v0.8.45 using helm chart 0.2.106

Looks like curl still gets -k option on our installation, it's weird

pedro93 added a commit that referenced this pull request Sep 29, 2022
* refactor(elasticsearch-setup-job): create-indices.sh readability

The script contains many copy-pasting and is not easy to follow.
Add comments, extract commonly used operations into functions, unify approaches.

* fix(elasticsearch-setup-job): AWS indices creation

Fix the issue where Amazon OpenSearch (AWS ES) indices are incorrectly initialised
and the Analytics screen shows errors only.

* feat(elasticsearch-setup-job): configuration hint

mention USE_AWS_ELASTICSEARCH env value if it seems it's set the wrong way

* fix(elasticsearch-setup-job): silent curl

* fix(elasticsearch-setup-job): better USE_AWS_ELASTICSEARCH hint

* docs(elasticsearch-setup-job): index dropping explained

- more comments
- more defensive approach
- index file renamed

* fix(elasticsearch-setup-job): script fixes

* merge(elasticsearch-setup-job): merging in PR #5937

* merge(elasticsearch-setup-job): merging in PR #5963

* merge(elasticsearch-setup-job): merging in PR #5975

Co-authored-by: Pedro Silva <[email protected]>
@davidspek davidspek mentioned this pull request Oct 5, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devops PR or Issue related to DataHub backend & deployment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants