Separating Upgrade test and ClickHouse Migration Tests #383
Conversation
tushartathgur
commented
Jul 18, 2023
•
tushartathgur
commented
- Latest changes in Antrea CRDs require us to delete the downgrade and check test, to keep it on par with Antrea, we have separated the upgrade test and the ClickHouse migration tests.
- In order to test for the data schema management, we have added new ClickHouse migration test.
Codecov Report
@@ Coverage Diff @@
## main #383 +/- ##
==========================================
- Coverage 66.13% 66.07% -0.06%
==========================================
Files 38 38
Lines 5049 5049
==========================================
- Hits 3339 3336 -3
- Misses 1553 1557 +4
+ Partials 157 156 -1
*This pull request uses carry forward flags. Click here to find out more. |
tushartathgur
force-pushed
the
upgrade_test
branch
7 times, most recently
from
88ae5f5
to
61605a7
Compare
tushartathgur
changed the title
Latest changes in Antrea CRDs require us to delete the downgrade and check test, to keep it on par with Antrea, we have separated the upgrade test and the ClickHouse migration tests. In order to test for the data Schema management, we have added new ClickHouse migration test. Signed-off-by: Tushar Tathgur <[email protected]>
tushartathgur
force-pushed
the
upgrade_test
branch
from
61605a7
to
fef4c2b
Compare
build/charts/theia/values.yaml
Outdated
| @@ -249,7 +249,7 @@ sparkOperator: | |||
| tag: "v1beta2-1.3.3-3.1.1" | |||
| theiaManager: | |||
| # -- Determine whether to install Theia Manager. | |||
| enable: true | |||
| enable: false | |||
There was a problem hiding this comment.
If you would like to exclude theia-manager from the ClickHouse upgrading test, I would suggest to add an option in generate manifest script for this. Maybe add an option clickhouse-only, so that we can exclude other components in case there are more in the future.
We would like to enable theia-manager by default as theia CLI has dependency on it and we would like to provide users this default functionality.
build/yamls/flow-visibility.yml
Outdated
| @@ -7282,7 +6965,7 @@ spec: | |||
| value: 1m | |||
| - name: SKIP_ROUNDS_NUM | |||
| value: "3" | |||
| image: projects.registry.vmware.com/antrea/theia-clickhouse-monitor:latest | |||
| image: projects.registry.vmware.com/antrea/antrea-ubuntu:latest | |||
There was a problem hiding this comment.
Why we have antrea image for clickhouse monitor?
There was a problem hiding this comment.
tbh I have no idea why it did that as it is a auto generated file and Values.yaml file was not changed for this to happen!
ci/kind/test-upgrade-theia.sh
Outdated
| docker exec -i kind-control-plane dd of=/root/clickhouse-operator-install-bundle.yaml < build/charts/theia/crds/clickhouse-operator-install-bundle.yaml | ||
| ./hack/generate-manifest.sh --mode release --local /data/clickhouse --no-grafana | docker exec -i kind-control-plane dd of=/root/flow-visibility-ch-only.yml |
There was a problem hiding this comment.
As this is the upgrade test for the whole theia, maybe we can add other components like grafana, spark operator, and theia manager?
ci/kind/test-clickhouse-migrate.sh
Outdated
| pushd $TMP_DIR > /dev/null | ||
| export IMG_NAME=projects.registry.vmware.com/antrea/antrea-ubuntu | ||
| export IMG_TAG=$ANTREA_FROM_TAG | ||
| ./hack/generate-manifest.sh --mode release | docker exec -i kind-control-plane dd of=/root/antrea.yml | ||
| popd | ||
| rm -rf $TMP_DIR |
There was a problem hiding this comment.
Since we plan to use the same Antrea yaml file for the ClickHouse migration test, maybe we can remove all the contents related to ANTREA_FROM_TAG in this file?
There was a problem hiding this comment.
We will require the ANTREA_FROM_TAG for installing the last release version of Theia for checking the upgrade from a release to the latest code. Similar arguments hold for THEIA_FROM_TAG ymls
ci/kind/test-clickhouse-migrate.sh
Outdated
| @@ -0,0 +1,262 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
I'm not good at naming. But maybe test-migrate-clickhouse.sh will be better considering the consistency?
tushartathgur
force-pushed
the
upgrade_test
branch
from
befbb8c
to
0977543
Compare
Signed-off-by: Tushar Tathgur <[email protected]>
tushartathgur
force-pushed
the
upgrade_test
branch
from
0977543
to
ed3fe31
Compare
Signed-off-by: Tushar Tathgur <[email protected]>
