Skip to content

Commit

Permalink
even more code review changes
Browse files Browse the repository at this point in the history
Signed-off-by: nadav mizrahi <[email protected]>
  • Loading branch information
nadavMiz committed Jul 3, 2023
1 parent e7abacf commit 7e73407
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 41 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ceph-s3-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
# Freeze the version of operator
# to avoid a failed run due to code changes in the operator repo.
# Need to update the commit once in a while
ref: f7358501716816250702d9a4c96f2526f98534e7
ref: ca12ff9e360220bb50bb9a2b645846e4b241fa39

- name: Change settings for k8s and minikube
run: |
Expand Down
63 changes: 31 additions & 32 deletions docs/dev_guide/ceph_s3_tests/ceph_s3_tests_guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ kubectl apply -f src/test/system_tests/ceph_s3_tests/test_ceph_s3_job.yml
kubectl logs job/noobaa-tests-s3 -f
```

Running all the tests on a local machine takes about 20 minutes. With the current setting, all tests should pass, but there are cases where the endpoint restarts and causes a test to fail. You can also run a single test to be sure that it passes.

#### Skipped tests

We run all the tests except the tests that appear in the lists `src/test/system_tests/ceph_s3_tests/s3-tests-lists` if you would like to add or remove a test you can edit those files (and then repeat the steps starting from 'Build Core And Tester Images (Noobaa-Core)' above).
Expand All @@ -101,17 +103,6 @@ Some tests are marked to be skipped in the code of ceph/s3-tests repository. Usu
In the test code the function:
`pytest.skip("")` will mark them to be skipped.

#### Boto3 tests
As mentioned in the [readme of ceph/s3-tests](https://github.com/ceph/s3-tests#readme):

Most of the tests have both Boto3 and Boto2 versions. Tests written in Boto2 are in the s3tests directory. Tests written in Boto3 are located in the s3test_boto3 directory.

In order to only run the boto3 version, tests in the boto directory run with `-- -m 'not fails_on_rgw`:

```bash
S3TEST_CONF=your.conf tox -- -m 'not fails_on_rgw' s3tests_boto3/functional::test_name
```

## Run a Single Ceph S3 Test

### 1) Prerequisites:
Expand Down Expand Up @@ -159,28 +150,12 @@ Run the script that will create the necessary accounts in noobaa and update the
node ./src/test/system_tests/ceph_s3_tests/test_ceph_s3_config_setup.js
```

#### Disable pytest warnings
If you want to disable summary warnings add the following flag to the test command:
`-- --disable-pytest-warnings`

For example:

```bash
S3TEST_CONF=${PWD}/src/test/system_tests/ceph_s3_tests/test_ceph_s3_config.conf tox -c src/test/system_tests/ceph_s3_tests/s3-tests/tox.ini -- --disable-pytest-warnings ${PWD}/src/test/system_tests/ceph_s3_tests/s3-tests/s3tests/functional/test_headers.py::test_bucket_create_contentlength_none
```

Note that every flag that comes after `--` is passed to pytest from tox. so if there is already `--` in the command just put `--disable-pytest-warnings` as part of the flags after it, no need to add another `--` notation.

for example to add --disable-pytest-warnings to the command:
```bash
S3TEST_CONF=${PWD}/src/test/system_tests/ceph_s3_tests/test_ceph_s3_config.conf tox -c src/test/system_tests/ceph_s3_tests/s3-tests/tox.ini -- -m 'not fails_on_rgw' ${PWD}/src/test/system_tests/ceph_s3_tests/s3-tests/s3tests_boto3/functional/test_s3.py::test_account_usage
```

it should be:
```bash
S3TEST_CONF=${PWD}/src/test/system_tests/ceph_s3_tests/test_ceph_s3_config.conf tox -c src/test/system_tests/ceph_s3_tests/s3-tests/tox.ini -- -m 'not fails_on_rgw --disable-pytest-warnings ${PWD}/src/test/system_tests/ceph_s3_tests/s3-tests/s3tests_boto3/functional/test_s3.py::test_account_usage
```
### 5) Run a Test (Inside The Tester Pod)
To run a test, from noobaa working directory:
```bash
Expand All @@ -193,6 +168,23 @@ You can find a list of tests in the doc inside the file `ceph_s3_tests_list_sing

In case the test name is incorrect, for example if you add `:` instead of `::` to the test name, the command will fail.
The error will be `file or directory not found` and pytest will exit with error code 4 (which means "pytest command line usage error")

#### Disable pytest warnings
If you want to disable summary warnings add the following flag to the test command:
`-- --disable-pytest-warnings`

Note that every flag that comes after `--` is passed to pytest from tox. so if there is already `--` in the command just put `--disable-pytest-warnings` as part of the flags after it, no need to add another `--` notation.

for example to add --disable-pytest-warnings to the command:
```bash
S3TEST_CONF=${PWD}/src/test/system_tests/ceph_s3_tests/test_ceph_s3_config.conf tox -c src/test/system_tests/ceph_s3_tests/s3-tests/tox.ini -- -m 'not fails_on_rgw' ${PWD}/src/test/system_tests/ceph_s3_tests/s3-tests/s3tests_boto3/functional/test_s3.py::test_account_usage
```

it should be:
```bash
S3TEST_CONF=${PWD}/src/test/system_tests/ceph_s3_tests/test_ceph_s3_config.conf tox -c src/test/system_tests/ceph_s3_tests/s3-tests/tox.ini -- -m 'not fails_on_rgw --disable-pytest-warnings ${PWD}/src/test/system_tests/ceph_s3_tests/s3-tests/s3tests_boto3/functional/test_s3.py::test_account_usage
```
## Debug a Single Test (Inside The Tester Pod)
### 1) Prerequisites:
Expand All @@ -205,15 +197,18 @@ The best place to start investigating is noobaa endpoint pod logs. if you are ru
### 3) Change a Test
Sometimes you would like to change a test: add printing of variables, skip an assertion as needed, or you suspect that it has a faulty and you would like to change the code.
#### A. Permanent change - this change will be saved in a repo, it is for continues investigating.
#### A. Temporary change - this change will be saved in the file inside the container, useful when you need a small change.
You can edit a test by going to the test file and editing the test function. See [View The Test Content](#2-view-the-test-content) for how to find the test function.
#### B. Permanent change - this change will be saved in a repo, it is for continues investigating.
1) Fork and clone the repository [ceph/s3-test](https://github.com/ceph/s3-tests).
2) Create a new branch from the hash number that was set in the file `./src/test/system_tests/ceph_s3_tests/test_ceph_s3_deploy.sh`.
3) Change the code, commit, and push to the remote branch.
4) Inside the file `test_ceph_s3_deploy.sh` (mentioned above) Change the values of `CEPH_LINK` to your remote repository and the `CEPH_TESTS_VERSION` to the newest commit in your repository.
5) Build the tester image again, deploy noobaa, and run the test (repeat the steps starting from 'Build Core And Tester Images (Noobaa-Core)' above).
#### B. Temporary change - this change will be saved in the file inside the container, useful when you need a small change.
You can edit a test by going to the test file and editing the test function. e.g. if you are working on test `s3tests_boto3.functional.test_s3:test_set_bucket_tagging` then you should `vim ./src/test/system_tests/ceph_s3_tests/s3-tests/s3tests_boto3/functional/test_s3.py` and search for the function `test_set_bucket_tagging`.
## Compare to AWS Response (Inside Tester Pod)
Prerequisites:
Following the 'Run a Single Ceph S3 Test' steps until 'Deploy The Tester Deployment (Noobaa-Core Tab)'.
Expand All @@ -228,7 +223,11 @@ In this section we will do some manual changes that will allow you to check AWS
vim src/test/system_tests/ceph_s3_tests/test_ceph_s3_config_aws.conf
```
* host = s3.amazonaws.com
* bucket prefix = choose_name (for example: `bucket prefix = foo-bucket` In case the test will fail to delete the bucket, you will need to manually delete it from AWS, and its name will be `foo-bucket1`, it adds suffix of 1).
* bucket prefix = choose_name
For example:
`bucket prefix = foo-bucket` In case the test will fail to delete the bucket, you will need to manually delete it from AWS, and its name will be `foo-bucket1`, it adds suffix of 1.
* access_key, secret_key appears 3 times each in the file.
3) Running tests with the new configuration files will run against AWS:
```bash
Expand Down Expand Up @@ -270,7 +269,7 @@ S3TEST_CONF=${PWD}/src/test/system_tests/ceph_s3_tests/test_ceph_s3_config.conf
Note that there is the warning:
`WARNING: could not copy distfile to //.tox/distshare`
this warning is for tox to use the same dependancies between projects. this feature is deprecated and not used on this project. In order to remove the warning you can modify src/test/system_tests/ceph_s3_tests/s3-tests/tox.ini to include the following line in the `[tox]` section: `distshare = /root/node_modules/noobaa-core/.tox/distshare`
this warning is for tox to use the same dependancies between projects. this feature is deprecated and not used on this project. In order to remove the warning you can modify `src/test/system_tests/ceph_s3_tests/s3-tests/tox.ini` to include the following line in the `[tox]` section: `distshare = /root/node_modules/noobaa-core/.tox/distshare`
### 2) Test Fail
For example:
Expand Down
9 changes: 3 additions & 6 deletions src/test/system_tests/ceph_s3_tests/test_ceph_s3.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const dbg = require('../../../util/debug_module')(__filename);
dbg.set_process_name('test_ceph_s3');
const argv = require('minimist')(process.argv.slice(2));
delete argv._;
const { CEPH_TEST, DEFAULT_NUMBER_OF_WORKERS, TOX_ARGS, AWS4_TEST_ENDING } = require('./test_ceph_s3_constants.js');
const { CEPH_TEST, DEFAULT_NUMBER_OF_WORKERS, TOX_ARGS, AWS4_TEST_SUFFIX } = require('./test_ceph_s3_constants.js');

const testing_status = {
pass: [],
Expand Down Expand Up @@ -101,15 +101,12 @@ async function test_worker() {

async function run_single_test(test) {
let ceph_args = `S3TEST_CONF=${process.cwd()}/${CEPH_TEST.test_dir}${CEPH_TEST.ceph_config}`;
if (test.endsWith(AWS4_TEST_ENDING)) {
if (test.endsWith(AWS4_TEST_SUFFIX)) {
ceph_args += ` S3_USE_SIGV4=true`;
}
let base_cmd = `${ceph_args} tox ${TOX_ARGS}`;
const base_cmd = `${ceph_args} tox ${TOX_ARGS}`;
if (!S3_CEPH_TEST_OUT_OF_SCOPE_REGEXP.test(test)) {
try {
if (test.includes('boto')) {
base_cmd = `${ceph_args} tox ${TOX_ARGS} -- -m 'not fails_on_rgw'`;
}
const full_test_command = `${base_cmd} ${process.cwd()}/${CEPH_TEST.test_dir}${CEPH_TEST.s3_test_dir}${test}`;
const res = await os_utils.exec(full_test_command, { ignore_rc: false, return_stdout: true });
if (res.includes('skipped')) {
Expand Down
4 changes: 2 additions & 2 deletions src/test/system_tests/ceph_s3_tests/test_ceph_s3_constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ const DEFAULT_NUMBER_OF_WORKERS = 5; //5 was the number of workers in the previo

const TOX_ARGS = `-c ${CEPH_TEST.test_dir}${CEPH_TEST.s3_test_dir}${CEPH_TEST.tox_config}`;

const AWS4_TEST_ENDING = '_aws4';
const AWS4_TEST_SUFFIX = '_aws4';

exports.CEPH_TEST = CEPH_TEST;
exports.DEFAULT_NUMBER_OF_WORKERS = DEFAULT_NUMBER_OF_WORKERS;
exports.TOX_ARGS = TOX_ARGS;
exports.AWS4_TEST_ENDING = AWS4_TEST_ENDING;
exports.AWS4_TEST_SUFFIX = AWS4_TEST_SUFFIX;

0 comments on commit 7e73407

Please sign in to comment.