You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2020/07/22 15:45:49 UTC
[GitHub] [ignite] timoninmaxim opened a new pull request #8071: Make ducktape work with ISE / ISP
timoninmaxim opened a new pull request #8071:
URL: https://github.com/apache/ignite/pull/8071
Features:
1. Modify tests to make it possible specify externally:
a. Ignite version, jvm_options and other custom ducktape options to test;
b. Build custom docker image;
c. Specify templates for Ignite configs;
2. Separate IgniteServer and IgniteClient config.
Fixes:
1. Use bind mount instead of volume to increase performance;
2. Clean ducker containers instead of all possible containers;
3. Remove obsolete code with check containers with ducker labels;
4. get_version now works for strings too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r460029985
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_config.py
##########
@@ -17,71 +17,49 @@
This module renders Ignite config and all related artifacts
"""
+from jinja2 import FileSystemLoader, Environment
+
+import os
+
+DEFAULT_CONFIG_PATH = os.path.dirname(os.path.abspath(__file__)) + "/config"
+DEFAULT_IGNITE_CONF = DEFAULT_CONFIG_PATH + "/ignite.xml.j2"
+
+
+class Config(object):
Review comment:
IgniteConfig?
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -44,11 +44,20 @@ class IgniteAwareService(BackgroundThreadService):
def __init__(self, context, num_nodes, version, properties):
super(IgniteAwareService, self).__init__(context, num_nodes)
+ if 'project' in context.globals:
+ self.path = IgnitePath(context.globals['project'])
+ else:
+ self.path = IgnitePath()
Review comment:
Can we resolve this inside IgnitePath?
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -44,11 +44,20 @@ class IgniteAwareService(BackgroundThreadService):
def __init__(self, context, num_nodes, version, properties):
super(IgniteAwareService, self).__init__(context, num_nodes)
+ if 'project' in context.globals:
+ self.path = IgnitePath(context.globals['project'])
+ else:
+ self.path = IgnitePath()
+
+ if 'jvm_opts' in context.globals:
+ self.jvm_options = context.globals['jvm_opts']
+ else:
+ self.jvm_options = ""
Review comment:
Seems, you may just always read from context.globals['jvm_opts']?
##########
File path: modules/ducktests/tests/docker/run_tests.sh
##########
@@ -16,15 +16,29 @@
# limitations under the License.
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+# Num of containers that ducktape will prepare for tests
IGNITE_NUM_CONTAINERS=${IGNITE_NUM_CONTAINERS:-11}
+# Path for tests to start
TC_PATHS=${TC_PATHS:-./ignitetest/}
+# Docker image name that ducktape will use to prepare containers
+IMAGE_NAME=${IMAGE_NAME}
die() {
echo $@
exit 1
}
if ${SCRIPT_DIR}/ducker-ignite ssh | grep -q '(none)'; then
- ${SCRIPT_DIR}/ducker-ignite up -n "${IGNITE_NUM_CONTAINERS}" || die "ducker-ignite up failed"
+ # If image name is specified that skip build and just pull it
+ if [ "$IMAGE_NAME" != "" ]; then
+ IMAGE_NAME=" --skip-build-image $IMAGE_NAME"
+ fi
+ ${SCRIPT_DIR}/ducker-ignite up -n "${IGNITE_NUM_CONTAINERS}" ${IMAGE_NAME} || die "ducker-ignite up failed"
+fi
+
+if [ "$VERSION" != "" ]; then
+ export _DUCKTAPE_OPTIONS="$_DUCKTAPE_OPTIONS --parameters '{\"version\":\"${VERSION}\"}'"
Review comment:
Let's just pass _DUCKTAPE_OPTIONS (btw, what's the reason to start a variable name with _?)
##########
File path: modules/ducktests/tests/ignitetest/tests/spark_integration_test.py
##########
@@ -28,38 +31,37 @@ class SparkIntegrationTest(IgniteTest):
3. Checks results of client application.
"""
- @staticmethod
- def properties(client_mode="false"):
- return """
- <property name="clientMode" value="{client_mode}"/>
- """.format(client_mode=client_mode)
-
def __init__(self, test_context):
super(SparkIntegrationTest, self).__init__(test_context=test_context)
- self.spark = SparkService(test_context, num_nodes=2)
- self.ignite = IgniteService(test_context, num_nodes=1)
+ self.spark = None
+ self.ignite = None
def setUp(self):
- self.spark.start()
- self.ignite.start()
+ pass
def teardown(self):
self.spark.stop()
self.ignite.stop()
- def test_spark_client(self):
+ @parametrize(version=str(DEV_BRANCH))
+ def test_spark_client(self, version):
+ self.spark = SparkService(self.test_context, version=version, num_nodes=2)
+ self.spark.start()
+
+ self.ignite = IgniteService(self.test_context, version=version, num_nodes=1)
+ self.ignite.start()
+
self.stage("Starting sample data generator")
- IgniteApplicationService(
- self.test_context,
- java_class_name="org.apache.ignite.internal.ducktest.tests.spark_integration_test.SampleDataStreamerApplication",
- params="cache,1000",
- properties=self.properties(client_mode="true")).run()
+ IgniteApplicationService(self.test_context,
+ java_class_name="org.apache.ignite.internal.ducktest.tests.spark_integration_test.SampleDataStreamerApplication",
+ params="cache,1000",
+ version=version).run()
self.stage("Starting Spark application")
- SparkIgniteApplicationService(
- self.test_context,
- "org.apache.ignite.internal.ducktest.tests.spark_integration_test.SparkApplication",
- params="spark://" + self.spark.nodes[0].account.hostname + ":7077",
- timeout_sec=120).run()
+ SparkIgniteApplicationService(self.test_context,
+ "org.apache.ignite.internal.ducktest.tests.spark_integration_test.SparkApplication",
+ params="spark://" + self.spark.nodes[0].account.hostname + ":7077",
+ version=version,
+ timeout_sec=120).run()
Review comment:
Let's keep shift to the left
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r459284580
##########
File path: modules/ducktests/.gitignore
##########
@@ -0,0 +1,2 @@
+tests/venv/
Review comment:
I believe that there is no need to add .gitignore here at all.
And I strongly advice you to use virtualenvwrapper in order do maintain different venv's.
It is recommended way in python community to use venv's in developer's environment.
If you used it it would not be necessary add something to .gitignore.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r459305186
##########
File path: modules/ducktests/tests/README.md
##########
@@ -0,0 +1,14 @@
+## Overview
+The `ignitetest` framework provides basic functionality and services
+to write integration tests for Apache Ignite. This framework bases on
Review comment:
I agree and tried use passive voice but Intellij Idea recommends to use active voice here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r459462429
##########
File path: modules/ducktests/tests/README.md
##########
@@ -0,0 +1,14 @@
+## Overview
+The `ignitetest` framework provides basic functionality and services
+to write integration tests for Apache Ignite. This framework bases on
+the `ducktape` test framework, for information about it check the links:
+- https://github.com/confluentinc/ducktape - source code of the `ducktape`;
+- http://ducktape-docs.readthedocs.io - documentation to the `ducktape`.
+
+Structure of the `ignitetest` directory is:
+- `./ignitetest/services` contains basic services functionality;
+- `./ignitetest/version.py` contains utils for versioning;
Review comment:
relocated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r460705894
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_config.py
##########
@@ -17,71 +17,49 @@
This module renders Ignite config and all related artifacts
"""
+from jinja2 import FileSystemLoader, Environment
+
+import os
+
+DEFAULT_CONFIG_PATH = os.path.dirname(os.path.abspath(__file__)) + "/config"
+DEFAULT_IGNITE_CONF = DEFAULT_CONFIG_PATH + "/ignite.xml.j2"
+
+
+class Config(object):
Review comment:
the filename is ignite_config, but the class name is just config.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r460050850
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -44,11 +44,20 @@ class IgniteAwareService(BackgroundThreadService):
def __init__(self, context, num_nodes, version, properties):
super(IgniteAwareService, self).__init__(context, num_nodes)
+ if 'project' in context.globals:
+ self.path = IgnitePath(context.globals['project'])
+ else:
+ self.path = IgnitePath()
Review comment:
We can just call IgnitePath(context.globals.get('project'))
Get returns None by default when key is absent.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r460530589
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_config.py
##########
@@ -17,71 +17,49 @@
This module renders Ignite config and all related artifacts
"""
+from jinja2 import FileSystemLoader, Environment
+
+import os
+
+DEFAULT_CONFIG_PATH = os.path.dirname(os.path.abspath(__file__)) + "/config"
+DEFAULT_IGNITE_CONF = DEFAULT_CONFIG_PATH + "/ignite.xml.j2"
+
+
+class Config(object):
Review comment:
It has nothing Ignite specific. Just render any jinja2 templare.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r459284814
##########
File path: modules/ducktests/tests/README.md
##########
@@ -0,0 +1,14 @@
+## Overview
+The `ignitetest` framework provides basic functionality and services
+to write integration tests for Apache Ignite. This framework bases on
Review comment:
s/bases on/is based on/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r460056626
##########
File path: modules/ducktests/tests/docker/run_tests.sh
##########
@@ -16,15 +16,29 @@
# limitations under the License.
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+# Num of containers that ducktape will prepare for tests
IGNITE_NUM_CONTAINERS=${IGNITE_NUM_CONTAINERS:-11}
+# Path for tests to start
TC_PATHS=${TC_PATHS:-./ignitetest/}
+# Docker image name that ducktape will use to prepare containers
+IMAGE_NAME=${IMAGE_NAME}
die() {
echo $@
exit 1
}
if ${SCRIPT_DIR}/ducker-ignite ssh | grep -q '(none)'; then
- ${SCRIPT_DIR}/ducker-ignite up -n "${IGNITE_NUM_CONTAINERS}" || die "ducker-ignite up failed"
+ # If image name is specified that skip build and just pull it
+ if [ "$IMAGE_NAME" != "" ]; then
+ IMAGE_NAME=" --skip-build-image $IMAGE_NAME"
+ fi
+ ${SCRIPT_DIR}/ducker-ignite up -n "${IGNITE_NUM_CONTAINERS}" ${IMAGE_NAME} || die "ducker-ignite up failed"
+fi
+
+if [ "$VERSION" != "" ]; then
+ export _DUCKTAPE_OPTIONS="$_DUCKTAPE_OPTIONS --parameters '{\"version\":\"${VERSION}\"}'"
Review comment:
I mean that DUCKTAPE_OPTIONS should contain --parameters as well and any other ducktape API params. No need to overcomplicate run_tests.sh API.
Just make DUCKTAPE_OPTIONS as you want (hardcode it %)) and export at wrapping sh.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r460743960
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_config.py
##########
@@ -17,71 +17,49 @@
This module renders Ignite config and all related artifacts
"""
+from jinja2 import FileSystemLoader, Environment
+
+import os
+
+DEFAULT_CONFIG_PATH = os.path.dirname(os.path.abspath(__file__)) + "/config"
+DEFAULT_IGNITE_CONF = DEFAULT_CONFIG_PATH + "/ignite.xml.j2"
+
+
+class Config(object):
Review comment:
there are descenders: IgniteServerConfig and IgniteClientConfig that use ignite config files. Config class could render any config
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r460037520
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -44,11 +44,20 @@ class IgniteAwareService(BackgroundThreadService):
def __init__(self, context, num_nodes, version, properties):
super(IgniteAwareService, self).__init__(context, num_nodes)
+ if 'project' in context.globals:
+ self.path = IgnitePath(context.globals['project'])
+ else:
+ self.path = IgnitePath()
+
+ if 'jvm_opts' in context.globals:
+ self.jvm_options = context.globals['jvm_opts']
+ else:
+ self.jvm_options = ""
Review comment:
It will raise KeyError in case that jvm_options aren't specified.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r459315860
##########
File path: modules/ducktests/tests/docker/ducker-ignite
##########
@@ -60,11 +60,18 @@ Usage: ${script_path} [command] [options]
help|-h|--help
Display this help message
-up [-n|--num-nodes NUM_NODES] [-f|--force] [docker-image]
- [-C|--custom-ducktape DIR] [-e|--expose-ports ports]
+build [-j|--jdk JDK] [image-name]
+ Build an docker image that ducktape uses to up cluster. Image is tagged with specified ${image_name}.
Review comment:
I see such pipeline: build - up - run. Tests are running on containers on final stage while image is used on "up" stage. I think I should make replacement to make it clearer. Other docs use term "node", so let's use it too:
> docker image that ducktape uses to up cluster.
to
> docker image that represents a ducker node.
##########
File path: modules/ducktests/tests/docker/ducker-ignite
##########
@@ -60,11 +60,18 @@ Usage: ${script_path} [command] [options]
help|-h|--help
Display this help message
-up [-n|--num-nodes NUM_NODES] [-f|--force] [docker-image]
- [-C|--custom-ducktape DIR] [-e|--expose-ports ports]
+build [-j|--jdk JDK] [image-name]
+ Build an docker image that ducktape uses to up cluster. Image is tagged with specified ${image_name}.
Review comment:
I see such pipeline: build - up - run. Tests are running on containers on final stage while image is used on "up" stage. I think I should make replacement to make it clearer. Other docs use term "node", so let's use it too:
> docker image that ducktape uses to up cluster.
to
> docker image that represents a ducker node.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r460531443
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -44,11 +44,20 @@ class IgniteAwareService(BackgroundThreadService):
def __init__(self, context, num_nodes, version, properties):
super(IgniteAwareService, self).__init__(context, num_nodes)
+ if 'project' in context.globals:
+ self.path = IgnitePath(context.globals['project'])
+ else:
+ self.path = IgnitePath()
+
+ if 'jvm_opts' in context.globals:
+ self.jvm_options = context.globals['jvm_opts']
+ else:
+ self.jvm_options = ""
Review comment:
fixed
##########
File path: modules/ducktests/tests/docker/run_tests.sh
##########
@@ -16,15 +16,29 @@
# limitations under the License.
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+# Num of containers that ducktape will prepare for tests
IGNITE_NUM_CONTAINERS=${IGNITE_NUM_CONTAINERS:-11}
+# Path for tests to start
TC_PATHS=${TC_PATHS:-./ignitetest/}
+# Docker image name that ducktape will use to prepare containers
+IMAGE_NAME=${IMAGE_NAME}
die() {
echo $@
exit 1
}
if ${SCRIPT_DIR}/ducker-ignite ssh | grep -q '(none)'; then
- ${SCRIPT_DIR}/ducker-ignite up -n "${IGNITE_NUM_CONTAINERS}" || die "ducker-ignite up failed"
+ # If image name is specified that skip build and just pull it
+ if [ "$IMAGE_NAME" != "" ]; then
+ IMAGE_NAME=" --skip-build-image $IMAGE_NAME"
+ fi
+ ${SCRIPT_DIR}/ducker-ignite up -n "${IGNITE_NUM_CONTAINERS}" ${IMAGE_NAME} || die "ducker-ignite up failed"
+fi
+
+if [ "$VERSION" != "" ]; then
+ export _DUCKTAPE_OPTIONS="$_DUCKTAPE_OPTIONS --parameters '{\"version\":\"${VERSION}\"}'"
Review comment:
fixed
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -44,11 +44,20 @@ class IgniteAwareService(BackgroundThreadService):
def __init__(self, context, num_nodes, version, properties):
super(IgniteAwareService, self).__init__(context, num_nodes)
+ if 'project' in context.globals:
+ self.path = IgnitePath(context.globals['project'])
+ else:
+ self.path = IgnitePath()
Review comment:
fixed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r459260414
##########
File path: modules/ducktests/tests/ignitetest/services/utils/config/ignite-client.xml.tmpl
##########
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
Review comment:
Use signle template for client/server
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r459305186
##########
File path: modules/ducktests/tests/README.md
##########
@@ -0,0 +1,14 @@
+## Overview
+The `ignitetest` framework provides basic functionality and services
+to write integration tests for Apache Ignite. This framework bases on
Review comment:
I agree and tried use passive voice but Intellij Idea recommends use active voice here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r460531357
##########
File path: modules/ducktests/tests/ignitetest/tests/spark_integration_test.py
##########
@@ -28,38 +31,37 @@ class SparkIntegrationTest(IgniteTest):
3. Checks results of client application.
"""
- @staticmethod
- def properties(client_mode="false"):
- return """
- <property name="clientMode" value="{client_mode}"/>
- """.format(client_mode=client_mode)
-
def __init__(self, test_context):
super(SparkIntegrationTest, self).__init__(test_context=test_context)
- self.spark = SparkService(test_context, num_nodes=2)
- self.ignite = IgniteService(test_context, num_nodes=1)
+ self.spark = None
+ self.ignite = None
def setUp(self):
- self.spark.start()
- self.ignite.start()
+ pass
def teardown(self):
self.spark.stop()
self.ignite.stop()
- def test_spark_client(self):
+ @parametrize(version=str(DEV_BRANCH))
+ def test_spark_client(self, version):
+ self.spark = SparkService(self.test_context, version=version, num_nodes=2)
+ self.spark.start()
+
+ self.ignite = IgniteService(self.test_context, version=version, num_nodes=1)
+ self.ignite.start()
+
self.stage("Starting sample data generator")
- IgniteApplicationService(
- self.test_context,
- java_class_name="org.apache.ignite.internal.ducktest.tests.spark_integration_test.SampleDataStreamerApplication",
- params="cache,1000",
- properties=self.properties(client_mode="true")).run()
+ IgniteApplicationService(self.test_context,
+ java_class_name="org.apache.ignite.internal.ducktest.tests.spark_integration_test.SampleDataStreamerApplication",
+ params="cache,1000",
+ version=version).run()
self.stage("Starting Spark application")
- SparkIgniteApplicationService(
- self.test_context,
- "org.apache.ignite.internal.ducktest.tests.spark_integration_test.SparkApplication",
- params="spark://" + self.spark.nodes[0].account.hostname + ":7077",
- timeout_sec=120).run()
+ SparkIgniteApplicationService(self.test_context,
+ "org.apache.ignite.internal.ducktest.tests.spark_integration_test.SparkApplication",
+ params="spark://" + self.spark.nodes[0].account.hostname + ":7077",
+ version=version,
+ timeout_sec=120).run()
Review comment:
fixed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r459260264
##########
File path: modules/ducktests/tests/ignitetest/services/utils/config/ignite-client.xml.tmpl
##########
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
Review comment:
Use .j2 file extension
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov merged pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
anton-vinogradov merged pull request #8071:
URL: https://github.com/apache/ignite/pull/8071
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r460036953
##########
File path: modules/ducktests/tests/docker/run_tests.sh
##########
@@ -16,15 +16,29 @@
# limitations under the License.
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+# Num of containers that ducktape will prepare for tests
IGNITE_NUM_CONTAINERS=${IGNITE_NUM_CONTAINERS:-11}
+# Path for tests to start
TC_PATHS=${TC_PATHS:-./ignitetest/}
+# Docker image name that ducktape will use to prepare containers
+IMAGE_NAME=${IMAGE_NAME}
die() {
echo $@
exit 1
}
if ${SCRIPT_DIR}/ducker-ignite ssh | grep -q '(none)'; then
- ${SCRIPT_DIR}/ducker-ignite up -n "${IGNITE_NUM_CONTAINERS}" || die "ducker-ignite up failed"
+ # If image name is specified that skip build and just pull it
+ if [ "$IMAGE_NAME" != "" ]; then
+ IMAGE_NAME=" --skip-build-image $IMAGE_NAME"
+ fi
+ ${SCRIPT_DIR}/ducker-ignite up -n "${IGNITE_NUM_CONTAINERS}" ${IMAGE_NAME} || die "ducker-ignite up failed"
+fi
+
+if [ "$VERSION" != "" ]; then
+ export _DUCKTAPE_OPTIONS="$_DUCKTAPE_OPTIONS --parameters '{\"version\":\"${VERSION}\"}'"
Review comment:
I've just reuse this variable, there is no reason to name it with '_'. Rename it.
I'm not sure that we should pass DUCKTAPE_OPTIONS as it json and there could be issues with bash + json. I'd like to prepare the options in run_tests.sh script and ask user to just fill env variables: VERSION, jvm_options, paths to config templates.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r460057666
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -44,11 +44,20 @@ class IgniteAwareService(BackgroundThreadService):
def __init__(self, context, num_nodes, version, properties):
super(IgniteAwareService, self).__init__(context, num_nodes)
+ if 'project' in context.globals:
+ self.path = IgnitePath(context.globals['project'])
+ else:
+ self.path = IgnitePath()
Review comment:
Anyway, we should do it inside IgnitePath to solve possible duplications in the future.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r459318424
##########
File path: modules/ducktests/tests/docker/ducker-ignite
##########
@@ -60,11 +60,18 @@ Usage: ${script_path} [command] [options]
help|-h|--help
Display this help message
-up [-n|--num-nodes NUM_NODES] [-f|--force] [docker-image]
- [-C|--custom-ducktape DIR] [-e|--expose-ports ports]
+build [-j|--jdk JDK] [image-name]
+ Build an docker image that ducktape uses to up cluster. Image is tagged with specified ${image_name}.
Review comment:
Please check, is it ok now?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r459260519
##########
File path: modules/ducktests/tests/ignitetest/version.py
##########
@@ -56,14 +58,17 @@ def get_version(node=None):
Return the version attached to the given node.
Default to DEV_BRANCH if node or node.version is undefined (aka None)
"""
- if node is not None and hasattr(node, "version") and node.version is not None:
- return node.version
- else:
- return DEV_BRANCH
+ if isinstance(node, ClusterNode) and hasattr(node, 'version') is not None:
Review comment:
hasattr never returns None
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r459286311
##########
File path: modules/ducktests/tests/docker/ducker-ignite
##########
@@ -60,11 +60,18 @@ Usage: ${script_path} [command] [options]
help|-h|--help
Display this help message
-up [-n|--num-nodes NUM_NODES] [-f|--force] [docker-image]
- [-C|--custom-ducktape DIR] [-e|--expose-ports ports]
+build [-j|--jdk JDK] [image-name]
+ Build an docker image that ducktape uses to up cluster. Image is tagged with specified ${image_name}.
Review comment:
s/an docker image/a docker image/
And is it correct that ducktape uses image? It uses a bunch of running containers, based on this image.
I think, that "Build a docker image for running tests." is a little bit better.
##########
File path: modules/ducktests/tests/docker/ducker-ignite
##########
@@ -75,6 +82,9 @@ up [-n|--num-nodes NUM_NODES] [-f|--force] [docker-image]
or a combination of port/port-range separated by comma (like 2181,9092 or 2181,5005-5008).
By default no port is exposed. See README.md for more detail on this option.
+ If --jdk is specified then we will use this argument as base image for ducker docker images.
Review comment:
I think that "for ducktest's docker image" is better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8071: Make ducktape work with ISE / ISP
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8071:
URL: https://github.com/apache/ignite/pull/8071#discussion_r460050071
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -44,11 +44,20 @@ class IgniteAwareService(BackgroundThreadService):
def __init__(self, context, num_nodes, version, properties):
super(IgniteAwareService, self).__init__(context, num_nodes)
+ if 'project' in context.globals:
+ self.path = IgnitePath(context.globals['project'])
+ else:
+ self.path = IgnitePath()
+
+ if 'jvm_opts' in context.globals:
+ self.jvm_options = context.globals['jvm_opts']
+ else:
+ self.jvm_options = ""
Review comment:
Python dict has method get(key, default)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org