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

Use elasticsearch operator 4.2, add workflow for 4.1 #631

Merged
merged 4 commits into from
Sep 11, 2019
Merged

Use elasticsearch operator 4.2, add workflow for 4.1 #631

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a12ebcf
Use Elasticsearch operator 4.2 by default and add CI for 4.1
pavolloffay Sep 11, 2019
7a365f0
Use kubectl
pavolloffay Sep 11, 2019
8cf4e74
Fix review comments
pavolloffay Sep 11, 2019
df137d9
Remove outdatted config for old SDK
pavolloffay Sep 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .github/workflows/e2e-openshift-4.1.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
name: "OpenShift end-to-end tests, ES operator 4.1"
on: [push, pull_request]

jobs:
end-to-end:
runs-on: ubuntu-latest
strategy:
matrix:
TEST_GROUP: [es-self-provisioned]
steps:
- uses: jpkrohling/setup-kubectl@v1-release
- uses: jpkrohling/setup-operator-sdk@v1-release
- uses: actions/checkout@v1
with:
path: src/github.com/jaegertracing/jaeger-operator # remove when using operator >= 0.9.0

- name: "setup docker"
run: ./.ci/setup-docker.sh

- name: "set max_map_count"
run: sudo sysctl -w vm.max_map_count=262144

- name: "start openshift"
run: ./.ci/start-openshift.sh

- name: "running end to end test"
env:
GOPATH: /home/runner/work/jaeger-operator #remove when using operator >= 0.9.0
CI: true
TEST_GROUP: ${{ matrix.TEST_GROUP }}
ES_OPERATOR_BRANCH: release-4.1
ES_OPERATOR_IMAGE: quay.io/openshift/origin-elasticsearch-operator:4.1
run: ./.ci/run-e2e-tests.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it run all e2e tests here, or just the es ones?

Copy link
Member Author

Choose a reason for hiding this comment

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

The original OCP workflow run es-self-provisioned test group. This file is just copied to run the same CI as before.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jpkrohling Thoughts - should it just be limited to run the self provisioned tests? and also update the other workflow aswell?

Copy link
Member Author

Choose a reason for hiding this comment

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

@objectiser It seems like a separate issue/discussion. Could you please create an issue to discuss which tests should be run on OCP?

Copy link
Contributor

Choose a reason for hiding this comment

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

should it just be limited to run the self provisioned tests? and also update the other workflow aswell?

Good question. I think I would still like to know if something is broken when using an externally managed ES. It might have been limited first for development purposes, and I probably forgot to enable it for the other tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am might be missing something, but it seems to me we are talking about different things here.

There is a test for externally managed elasticsearch - the group name is es. The question is whether we want to run it on openshift, at the moment we run it only on k8s.

I think externally managed elasticsearch test on openshift is not very important. However there might be other tests more suitable to run on Openshift e.g. test oauth proxy (I am not sure if there is such a test).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok will open a separate issue.

16 changes: 12 additions & 4 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ OPERATOR_VERSION ?= "$(shell git describe --tags)"
STORAGE_NAMESPACE ?= "${shell kubectl get sa default -o jsonpath='{.metadata.namespace}' || oc project -q}"
KAFKA_NAMESPACE ?= "kafka"
ES_OPERATOR_NAMESPACE ?= openshift-logging
ES_OPERATOR_BRANCH ?= release-4.1
ES_OPERATOR_IMAGE ?= quay.io/openshift/origin-elasticsearch-operator:4.1
SDK_VERSION=v0.10.0
ES_OPERATOR_BRANCH ?= release-4.2
ES_OPERATOR_IMAGE ?= quay.io/openshift/origin-elasticsearch-operator:4.2
SDK_VERSION=v0.8.1
GOPATH ?= "$(HOME)/go"

LD_FLAGS ?= "-X $(VERSION_PKG).version=$(OPERATOR_VERSION) -X $(VERSION_PKG).buildDate=$(VERSION_DATE) -X $(VERSION_PKG).defaultJaeger=$(JAEGER_VERSION)"
Expand Down Expand Up @@ -146,10 +146,18 @@ run-debug: CLI_FLAGS = "--log-level=debug"

.PHONY: set-max-map-count
set-max-map-count:
# This is not required in OCP 4.1. The node tuning operator configures the property automatically
# when label tuned.openshift.io/elasticsearch=true label is present on the ES pod. The label
# is configured by ES operator.
@minishift ssh -- 'sudo sysctl -w vm.max_map_count=262144' > /dev/null 2>&1 || true

.PHONY: set-node-os-linux
set-node-os-linux:
# Elasticsearch requires labeled nodes. These labels are by default present in OCP 4.2
@kubectl label nodes --all kubernetes.io/os=linux --overwrite

.PHONY: deploy-es-operator
deploy-es-operator: set-max-map-count
deploy-es-operator: set-node-os-linux set-max-map-count
ifeq ($(OLM),true)
@echo Skipping es-operator deployment, assuming it has been installed via OperatorHub
else
Expand Down