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

Elasticsearch tests #353

Merged
merged 18 commits into from
Feb 11, 2019
Merged

Elasticsearch tests #353

merged 18 commits into from
Feb 11, 2019

Conversation

khajduczenia
Copy link
Collaborator

No description provided.

@khajduczenia
Copy link
Collaborator Author

/gcbrun

@khajduczenia
Copy link
Collaborator Author

/gcbrun

@khajduczenia
Copy link
Collaborator Author

/gcbrun

stdout:
contains: OK - green status

- name: Add a document
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you name steps in a question form? Can add a document instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I rephrased the names.

curl -s -f "${ELASTIC_URL}/_search?q=jones"
expect:
stdout:
contains: "\"name\":\"John\""
Copy link
Contributor

Choose a reason for hiding this comment

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

EOF

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.


# Wait for green status
status="$(get_cluster_status)"
while [[ "${status}" != '"green"' ]] \
Copy link
Contributor

Choose a reason for hiding this comment

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

If you modify jq in get_cluster_status function as following: jqlang/jq#250, then you can drop ugly " here and below.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, it's nice. Corrected.

expected_nodes="${REPLICAS}"

timeout=300
timeout_time=$(( $(date +%s) + timeout ))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cluster might not be formed - do you mean not having timeouts in the test and rely on the deployer's timeout or Cloud Build's timeout? Then we rely on the execution environment for a container, which should rather be environment-agnostic.


# Wait for all nodes to join the cluster
nodes_count="$(get_nodes_count)"
while [[ "${nodes_count}" -ne "${expected_nodes}" ]] \
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a question: Does it mean that running Pod can be still not connected to the cluster?

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.

done

if [[ "${nodes_count}" -eq "${expected_nodes}" ]]; then
echo "OK - all nodes joined"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I prefer stdout checks for ease of debugging in case of failure, even if I agree that checking the exitCode is more readable. We can return to checking the exitCode once testrunner prints the logs from both stdout and stderr in case of failure.

done

if [[ "${nodes_count}" -eq "${expected_nodes}" ]]; then
echo "OK - all nodes joined"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I prefer stdout checks for ease of debugging in case of failure, even if I agree that checking the exitCode is more readable. We can return to checking the exitCode once testrunner prints the logs from both stdout and stderr in case of failure.

expected_nodes="${REPLICAS}"

timeout=300
timeout_time=$(( $(date +%s) + timeout ))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cluster might not be formed - do you mean not having timeouts in the test and rely on the deployer's timeout or Cloud Build's timeout? Then we rely on the execution environment for a container, which should rather be environment-agnostic.

@khajduczenia
Copy link
Collaborator Author

/gcbrun

@khajduczenia
Copy link
Collaborator Author

/gcbrun

@khajduczenia
Copy link
Collaborator Author

/gcbrun

@khajduczenia
Copy link
Collaborator Author

/gcbrun

@khajduczenia
Copy link
Collaborator Author

/gcbrun

fi
expect:
stdout:
equals: 'OK - all nodes found'
Copy link
Contributor

@wgrzelak wgrzelak Feb 9, 2019

Choose a reason for hiding this comment

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

JFYI: Because of GoogleCloudPlatform/marketplace-testrunner#6 change, we will be able to drop the if statement and replace equals with something like {{ .Env.REPLICAS }}

@khajduczenia khajduczenia merged commit a59e739 into master Feb 11, 2019
@khajduczenia khajduczenia deleted the elasticsearch-tests branch February 11, 2019 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants