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

Test with "external" elasticsearch #441

Merged
merged 6 commits into from
Jul 13, 2023

Conversation

csibbitt
Copy link
Collaborator

  • Strip all ES config except "enable" (for forwarding)
  • Create ECK subscription no matter the observability_strategy
  • Deploy ES from CI for events testing (Code copied/trimmed from STO)
  • Default to use_redhat for CI

* Strip all ES config except "enable" (for forwarding)
* Create ECK subscription no matter the observability_strategy
* Deploy ES from CI for events testing (Code copied/trimmed from STO)
* Default to use_redhat for CI
@csibbitt csibbitt added the do-not-merge Code is not ready to be merged label Jul 10, 2023
@csibbitt
Copy link
Collaborator Author

I haven't tested this. I was having trouble with the CatalogSources in my test cluster, so I'm sending it to the CI system instead. I don't really expect it to work yet, but maybe?

@csibbitt
Copy link
Collaborator Author

test

* ephemeral volume
* wait for CRD to establish
@csibbitt
Copy link
Collaborator Author

test

* Test events to external ES in any observability_strategy mode
* We no longer need the smoketest to know the observability_strategy at all
@csibbitt
Copy link
Collaborator Author

test

@csibbitt
Copy link
Collaborator Author

csibbitt commented Jul 12, 2023

This looks like it's working. I checked the smoketest logs from the 4.12 CI run and saw this:

*** [INFO] Get documents for this test from ElasticSearch...
*** [INFO] Found 34 documents

@csibbitt csibbitt removed the do-not-merge Code is not ready to be merged label Jul 12, 2023
@csibbitt
Copy link
Collaborator Author

4.12 CI testing passed
4.10 is failing to even build with this error

{"changed": false, "cmd": "/usr/bin/git checkout --force csibbitt/STF-1386_move_es_into_CI", "msg": "Failed to checkout csibbitt/STF-1386_move_es_into_CI", "rc": 1, "stderr": "error: pathspec 'csibbitt/STF-1386_move_es_into_CI' did not match any file(s) known to git\n", "stderr_lines": ["error: pathspec 'csibbitt/STF-1386_move_es_into_CI' did not match any file(s) known to git"], "stdout": "", "stdout_lines": []}

That's a bit weird, n'est ce pas?

Here it is working on my machine

$ git checkout --force csibbitt/STF-1386_move_es_into_CI
Switched to branch 'csibbitt/STF-1386_move_es_into_CI'
Your branch is up to date with 'origin/csibbitt/STF-1386_move_es_into_CI'.

Will recheck.

@csibbitt
Copy link
Collaborator Author

4.10 built this time but ES is crashing on start with this error:

ERROR: [1] bootstrap checks failed. You must address the points described in the following [1] lines before starting Elasticsearch.
bootstrap check failure [1] of [1]: max virtual memory areas vm.max_map_count [65530] is too low, increase to at least [262144]
ERROR: Elasticsearch did not exit normally - check the logs at /usr/share/elasticsearch/logs/elasticsearch.log

This is unexpected. I can see that value being set for minishift in an old test script [1] but I don't see anything in our main code that would have been setting this.

We explicitly enable mmap in our config[2][3], but never set this limit.

I'm tempted to force mmap off (node.store.allow_mmap: false) for CI testing purposes. We don't support the deployment or operation of Elasticsearch, so whether it's running without or without mmap optimizations isn't something I think should matter for our testing.

[1] https://github.com/infrawatch/service-telemetry-operator/blob/master/tests/infrared/13/baremetal-scripts/install-and-run-minishift.sh#L23
[2] https://github.com/infrawatch/service-telemetry-operator/blob/master/roles/servicetelemetry/templates/manifest_elasticsearch.j2#L26
[3] https://github.com/infrawatch/service-telemetry-operator/pull/441/files#diff-d8bb56aeb8cb496de1440f1d6ff33b7d35310db46cdf66df040c2f958936efffR25

@csibbitt
Copy link
Collaborator Author

According to our docs[1], the label[2] included in our pod template is supposed to take care of this. I suspect disruption in the CI cluster may be preventing this from working, but not sure yet.

[1] https://github.com/infrawatch/documentation/blob/master/doc-Service-Telemetry-Framework/modules/con_node-tuning-operator.adoc#L29
[2] https://github.com/infrawatch/service-telemetry-operator/pull/441/files#diff-d8bb56aeb8cb496de1440f1d6ff33b7d35310db46cdf66df040c2f958936efffR29

@csibbitt
Copy link
Collaborator Author

4.10 CI passed on retry with no changes.

This should be ready to go now.

@@ -34,15 +34,6 @@
events:
elasticsearch:
enabled: {{ __service_telemetry_events_enabled }}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In my last PR, Leif suggested I make the defaults match exactly what we currently deploy so that migrations will be easy. Since I just copied the template from the operator to the CI code, a side effect is that we don't need to set any values for testing purposes, just enable it and the forwarding is correctly configured.

@@ -119,3 +119,15 @@
kind: Project
metadata:
name: openshift-cert-manager-operator

- name: Remove elasticsearch
ignore_errors: True
Copy link
Collaborator Author

@csibbitt csibbitt Jul 13, 2023

Choose a reason for hiding this comment

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

ignore_errors is required because if there is no ECK installed the apiVersion will get rejected with an error.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure how fancy you want to get, but you could also add a when: query(...) | length != 0 to run this only when the API is available. This works too though :)

@@ -0,0 +1,52 @@
apiVersion: elasticsearch.k8s.elastic.co/v1
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

This will be good information for the KCS after forwarding becomes the supported interface

Copy link
Member

@leifmadsen leifmadsen left a comment

Choose a reason for hiding this comment

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

Couple of nitpicky comments, but this all looks sane to me

@@ -119,3 +119,15 @@
kind: Project
metadata:
name: openshift-cert-manager-operator

- name: Remove elasticsearch
ignore_errors: True
Copy link
Member

Choose a reason for hiding this comment

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

Not sure how fancy you want to get, but you could also add a when: query(...) | length != 0 to run this only when the API is available. This works too though :)

build/stf-run-ci/tasks/setup_elasticsearch.yml Outdated Show resolved Hide resolved
@@ -0,0 +1,52 @@
apiVersion: elasticsearch.k8s.elastic.co/v1
Copy link
Member

Choose a reason for hiding this comment

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

This will be good information for the KCS after forwarding becomes the supported interface

@@ -66,7 +64,7 @@ oc create configmap stf-smoketest-ceilometer-entrypoint-script --from-file "${RE
echo "*** [INFO] Creating smoketest jobs..."
oc delete job -l app=stf-smoketest
for NAME in "${CLOUDNAMES[@]}"; do
oc create -f <(sed -e "s/<<CLOUDNAME>>/${NAME}/;s/<<ELASTICSEARCH_AUTH_PASS>>/${ELASTICSEARCH_AUTH_PASS}/;s/<<PROMETHEUS_AUTH_PASS>>/${PROMETHEUS_AUTH_PASS}/;s/<<OBSERVABILITY_STRATEGY>>/${OBSERVABILITY_STRATEGY}/" ${REL}/smoketest_job.yaml.template)
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

build/stf-run-ci/defaults/main.yml Outdated Show resolved Hide resolved
Copy link
Collaborator

@vkmc vkmc left a comment

Choose a reason for hiding this comment

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

LGTM Chris, thanks

build/stf-run-ci/tasks/pre-clean.yml Outdated Show resolved Hide resolved
build/stf-run-ci/tasks/setup_base.yml Outdated Show resolved Hide resolved
tests/smoketest/smoketest_collectd_entrypoint.sh Outdated Show resolved Hide resolved
Co-authored-by: Leif Madsen <lmadsen@redhat.com>
build/stf-run-ci/defaults/main.yml Outdated Show resolved Hide resolved
build/stf-run-ci/templates/manifest_elasticsearch.j2 Outdated Show resolved Hide resolved
build/stf-run-ci/tasks/pre-clean.yml Outdated Show resolved Hide resolved
build/stf-run-ci/tasks/setup_base.yml Outdated Show resolved Hide resolved
tests/smoketest/smoketest_collectd_entrypoint.sh Outdated Show resolved Hide resolved
@csibbitt csibbitt merged commit 483f898 into master Jul 13, 2023
6 checks passed
@csibbitt csibbitt deleted the csibbitt/STF-1386_move_es_into_CI branch July 13, 2023 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants