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

Jump to bottom

Improve release docs and IT prechecks #914

Merged
merged 6 commits into from
Feb 28, 2020
Merged

Improve release docs and IT prechecks #914

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
96f63b6
Add check for minimum Docker memory on MacOS.
Feb 21, 2020
c91a1cb
Add 'make it' precondition to RELEASE.md
Feb 21, 2020
601eb0e
Move Docker memory check to integration-test.sh
Feb 21, 2020
b3b81f7
Merge branch 'master' into improve-release-docs
Feb 21, 2020
65070b7
Fall back to integer comparison since CI machines don't have 'bc'
Feb 21, 2020
43bb3b1
Perform Docker memory check in bytes with info --format
Feb 27, 2020
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
1 change: 1 addition & 0 deletions RELEASE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Please execute the following tasks after `make release` has finished:
### Preconditions

* Ensure that the master branch is checked out and your working copy is clean (run `git status`).
* Ensure that integration tests can run to completion successfully on your machine (run `make it`).
* Ensure that the associated milestone on Github contains no open tickets (otherwise the release will fail).

### Initial Setup
Expand Down
8 changes: 8 additions & 0 deletions integration-test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ readonly ES_METRICS_STORE_TRANSPORT_PORT="63200"
readonly ES_ARTIFACT_PATH="elasticsearch-${ES_METRICS_STORE_VERSION}"
readonly ES_ARTIFACT="${ES_ARTIFACT_PATH}.tar.gz"
readonly MIN_CURL_VERSION=(7 12 3)
readonly MIN_DOCKER_MEM_GB=6
readonly RALLY_LOG="${HOME}/.rally/logs/rally.log"
readonly RALLY_LOG_BACKUP="${HOME}/.rally/logs/rally.log.it.bak"

Expand All @@ -43,6 +44,13 @@ TEST_SUCCESS=0
function check_prerequisites {
exit_if_docker_not_running

DOCKER_MEM=$(docker info | grep Memory | awk '{print int($3 + 0)}')
Copy link
Member

Choose a reason for hiding this comment

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

If I specify exactly 6GB in the Docker Desktop UI (on Mac) this check fails.

The reason is that memory with docker info is reported in GiB instead of GB. I played around a bit with docker info and you can actually force it to show the raw number in bytes with docker info --format="{{.MemTotal}}". You can also use the expr utility to convert the GB value specified by MIN_DOCKER_MEM_GB to bytes with expr $MIN_DOCKER_MEM_GB \* 1024 \* 1024 \* 1024 and base the entire calculation on bytes.

One confusing aspect that I noticed is that I get 6237241344 (bytes) as output from docker info when specifying 6GB in the UI but 6GB converted to bytes is 6000000000 bytes so I assume their UI should actually label this as GiB which results in 6442450944 bytes and I further assume that this is the actual value used for the underlying VM but it reserves ~195MB for the OS and only provides 6237241344 bytes to Docker. I wonder whether we should allow for a small margin of - say 5% - in the actual check to account for that (even better would be to get the actual value specified in the UI of course)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is less important to check on the UI values, compared to the actual value available to the VM as this is what will break integration testing. The UI inconsistency is unfortunate, but ultimately out of scope here.

That said, the 6 GB minimum isn't as exact as the check would make it seem, but it works and I don't see much advantage to a margin here.

Copy link
Member

Choose a reason for hiding this comment

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

Given the target audience that's fine for me.

if [[ ${DOCKER_MEM} -lt ${MIN_DOCKER_MEM_GB} ]]; then
echo "Error: Docker is not configured with enough memory."
echo "Please increase memory available to Docker to at least ${MIN_DOCKER_MEM_GB} GB"
exit 1
fi

local curl_major_version=$(curl --version | head -1 | cut -d ' ' -f 2,2 | cut -d '.' -f 1,1)
local curl_minor_version=$(curl --version | head -1 | cut -d ' ' -f 2,2 | cut -d '.' -f 2,2)
local curl_patch_release=$(curl --version | head -1 | cut -d ' ' -f 2,2 | cut -d '.' -f 3,3)
Expand Down