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

macOS/arm64 e2e test improvements #5615

Merged
merged 31 commits into from Aug 31, 2022
Merged

macOS/arm64 e2e test improvements #5615

merged 31 commits into from Aug 31, 2022

Conversation

douglascamata
Copy link
Contributor

@douglascamata douglascamata commented Aug 19, 2022 •
edited

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

  • Upgraded MinIO to the highest release at the same that supported well ARM64.
    • Fixed deprecation warning regarding new env vars for user and password.
  • Changed MinIO's readiness check in e2e tests to be linked to the web console readiness. No more problems with MinIO's container reporting to be ready without being able to serve requests.
    If this works well, Minio is not ready even after StartAndWaitReady completes efficientgo/e2e#11 should be fixable and Thanos don't need its own function to set up MinIO for the e2e tests.
  • Fixed e2e test names that were generating invalid hostnames according to RFC 822 because MinIO refuses to run if the hostname is not compliant.
    This is a very interesting one. There are other limitations to hostnames, i.e. maximum 63 characters, shouldn't end in a period (.). Possibly something could be implemented in https://github.com/efficientgo/e2e with extra care to avoid a breaking change.
    Curiously all other software involved in the e2e tests doesn't seem to care about the underscores (_) that cause the non-compliance. I suppose Docker's networking is somehow managing it so it doesn't break things like DNS, but I am not sure.
  • Fixed bucket names, which according to MinIO's "strict" validations (compatible with S3 rules), do not allow underscores (_).
  • Moved away from special fork of Avalanche to the upstream repo.

Verification

Ran e2e tests a few times in both an ARM64 Linux and macOS boxes. This PR has a lot of valid improvements, but still didn't manage to get a full run of make e2e-test-local to pass.

macOS

Has issues with software to run Docker on Mac, causing random failures. Depending on the Docker platform being used, rerunning the failed tests will work.

Docker Desktop has issues running Avalanche. Rancher Desktop has issues with networking and ports. Podman can't build the Thanos container image.

Linux

Has some issues with TestToolsBucketWebExternalPrefixAndRoutePrefix.

@douglascamata
MinIO upgrade and fixes
d9c889b 
Start using the web console as readiness probe for e2e tests to ensure
MinIO  is fully ready to serve requests.

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Change all Docker env names with _ to -
e70455a 
The reason for this is that the name becomes part of the Docker
containers names.
These are also hostnames and as defined in RFC 822, hostnames can only
have alphanumeric characters, hyphen (`-`), and periods (`.`).
Some software supports hostnames with underscores (`_`), but MinIO
decided not to.

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Fix a missing invalid hostname
843856f 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Fix hostnames that are part of metrics
d7b8944 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Avoid underscore in minio's bucket name
0826ffe 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Replace underscore with hyphen in bucket name
0de1a58 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Replace underscore for hyphen in more buckets
f4c61f5 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Fix groupcache_group config in tests
ec6f4a0 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Copy link
Contributor Author

Waiting for efficientgo/e2e#45 to be merged to complete the ARM64 fixes.

@douglascamata
Stop using avalanche fork without multi-arch
9d0780d 
Also the issue that made it need is now officially merged.

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Upgrade MinIO in unused (?) file
89304f5 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Merge branch 'main' of github.com:douglascamata/thanos into arm64-e2e…
01e70c1 
…-test-fixes
@douglascamata
Copy link
Contributor Author

Was also waiting on efficientgo/e2e#46. Will soon update this and mark it ready for review.

@douglascamata
Bump version of efficientgo/e2e
1a5b398 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Merge branch 'main' of github.com:thanos-io/thanos into arm64-e2e-tes…
16810d6 
…t-fixes
@douglascamata douglascamata changed the title ARM64 e2e test fixes macOS/arm64 e2e test fixes Aug 24, 2022
@douglascamata
Fix deps
b127bfb 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Update to official avalanche repo
a406575 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Retrigger CI
5412255 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Retrigger CI
1be2fed 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Retrigger CI
db75457 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Update e2e test prometheus image
4b91f77 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Fixes after default Prometheus upgrade
60dd2c9 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Retrigger CI
d044585 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
More fixes related to target api test
001e31e 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Fix second target api test
383115e 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Fix another query test
42150f8 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Retrigger CI
634ba36 
Signed-off-by: GitHub <noreply@github.com>
@douglascamata
Fix receive e2e test
c31403c 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Remove prints and use logs
49c762a 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Fix broken logic in compact test
6a83a87 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Skip preinfo api compatbility test on arm64
7fa712b 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata
Improve multi-arch check
a37092f 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata douglascamata changed the title macOS/arm64 e2e test fixes macOS/arm64 e2e test improvements Aug 26, 2022
@douglascamata
Retrigger CI
b994cea 
Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
@douglascamata douglascamata marked this pull request as ready for review August 26, 2022 15:45
bwplotka
bwplotka approved these changes Aug 31, 2022
View reviewed changes
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Thanks!

@bwplotka bwplotka merged commit 739c876 into thanos-io:main Aug 31, 2022
prajain12 pushed a commit to prajain12/thanos that referenced this pull request Sep 6, 2022
@douglascamata @prajain12
macOS/arm64 e2e test improvements (thanos-io#5615)
677f1a1 
* MinIO upgrade and fixes

Start using the web console as readiness probe for e2e tests to ensure
MinIO  is fully ready to serve requests.

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Change all Docker env names with `_` to `-`

The reason for this is that the name becomes part of the Docker
containers names.
These are also hostnames and as defined in RFC 822, hostnames can only
have alphanumeric characters, hyphen (`-`), and periods (`.`).
Some software supports hostnames with underscores (`_`), but MinIO
decided not to.

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Fix a missing invalid hostname

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Fix hostnames that are part of metrics

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Avoid underscore in minio's bucket name

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Replace underscore with hyphen in bucket name

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Replace underscore for hyphen in more buckets

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Fix groupcache_group config in tests

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Stop using avalanche fork without multi-arch

Also the issue that made it need is now officially merged.

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Upgrade MinIO in unused (?) file

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Bump version of efficientgo/e2e

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Fix deps

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Update to official avalanche repo

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Retrigger CI

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Retrigger CI

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Retrigger CI

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Update e2e test prometheus image

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Fixes after default Prometheus upgrade

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Retrigger CI

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* More fixes related to target api test

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Fix second target api test

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Fix another query test

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Retrigger CI

Signed-off-by: GitHub <noreply@github.com>

* Fix receive e2e test

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Remove prints and use logs

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Fix broken logic in compact test

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Skip preinfo api compatbility test on arm64

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Improve multi-arch check

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Retrigger CI

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: Prakul Jain <prakul.jain@udaan.com>
@douglascamata douglascamata mentioned this pull request Oct 25, 2022
Closed
@douglascamata douglascamata mentioned this pull request Dec 2, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bwplotka bwplotka bwplotka approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@douglascamata @bwplotka