Skip to content

support for cassandra in integration tests #2275

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
Merged
merged 3 commits into from
Mar 18, 2020
Merged

support for cassandra in integration tests #2275

merged 3 commits into from
Mar 18, 2020

Conversation

sandeepsukhani
Copy link
Contributor

What this PR does:
Adds support of cassandra in integration tests

I have used an optimized docker image for integration tests. Here is the repo for it https://github.com/saidbouras/cassandra-docker-unit.
There is also a community maintained docker image https://hub.docker.com/_/cassandra, which is slow to start and takes about 10-15 additional seconds.
Cassandra version in image built by saidbouras is 3.11 while community build docker has 3.11.6 which I feel is okay. We can use community maintained docker image as well if we want if that is not okay.

Since Cassandra is slow to start and does not have an HTTP readiness probe, I have added support for command based probe. An example of readiness probe for Cassandra can be found here: https://github.com/kubernetes/examples/blob/b86c9d50be45eaf5ce74dee7159ce38b0e149d38/cassandra/image/files/ready-probe.sh

@sandeepsukhani sandeepsukhani force-pushed the cassandra-integration-test branch from 07411a5 to 2bff0ef Compare March 16, 2020 10:47
pstibrany
pstibrany approved these changes Mar 16, 2020
View reviewed changes
Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

LGTM, with minor nits.

integration/e2e/service.go Outdated
@@ -363,6 +390,20 @@ func (p *ReadinessProbe) Ready(localPort int) (err error) {
return fmt.Errorf("got no expected status code: %v, expected: %v", res.StatusCode, p.expectedStatus)
}

// ReadinessProbeCmd checks readiness by `Exec`ing a command which returns 0 to consider status being ready
Copy link
Contributor

Choose a reason for hiding this comment

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

executing or running a command (within container)

@sandeepsukhani
Copy link
Contributor Author

LGTM, with minor nits.

Thanks Peter for the review! I have pushed the changes.

@sandeepsukhani sandeepsukhani force-pushed the cassandra-integration-test branch from cda8063 to ea07148 Compare March 16, 2020 13:32
pracucci
pracucci reviewed Mar 18, 2020
View reviewed changes
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Very good job @sandeepsukhani! I left a comment and then we're good to go.

integration/e2e/service.go Outdated
func (p *ReadinessProbe) Ready(localPort int) (err error) {
func (p *HTTPReadinessProbe) Ready(service *ConcreteService) (err error) {
// Map the container port to the local port
localPort, ok := service.networkPortsContainerToLocal[p.port]
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be readiness probe's responsability. What do you think to add a function ConcreteService.EndpointPort() which is like .Endpoint() but just returns the port? We could return 0 if the port doesn't exist and convert the following if from if !ok to if port == 0.

What's your take?

Copy link
Contributor

Choose a reason for hiding this comment

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

Errata corrige. You need the endpoint here, so you can directly use Endpoint().

pracucci
pracucci approved these changes Mar 18, 2020
View reviewed changes
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@sandeepsukhani
support for cassandra in integration tests
1811ac6 
Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>
@sandeepsukhani
changes suggested from PR review
2654686 
Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>
@sandeepsukhani
using ConcreteService.Endpoint for getting http endpoint for readines…
cfa2253 
…s probe

Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>
@sandeepsukhani sandeepsukhani force-pushed the cassandra-integration-test branch from aa24c21 to cfa2253 Compare March 18, 2020 11:11
@pracucci pracucci merged commit bf59f9f into cortexproject:master Mar 18, 2020
pracucci added a commit to grafana/cortex that referenced this pull request Mar 19, 2020
@pracucci
Squashed commit of the following:
5ee1e6e 
commit 8e9c601
Author: Marco Pracucci <marco@pracucci.com>
Date:   Thu Mar 19 10:44:34 2020 +0100

    Avoid a superfluous querier integration test

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

commit 9691dc6
Author: Marco Pracucci <marco@pracucci.com>
Date:   Thu Mar 19 09:26:30 2020 +0100

    Separated metrics helpers unit tests using go blocks

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

commit d32afe8
Author: Marco Pracucci <marco@pracucci.com>
Date:   Thu Mar 19 09:23:34 2020 +0100

    Reworded util.StringsContain() comment and params

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

commit a5000a8
Author: Marco Pracucci <marco@pracucci.com>
Date:   Thu Mar 19 09:21:54 2020 +0100

    Update docs/operations/dns-service-discovery.md

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

    Co-Authored-By: Peter Štibraný <pstibrany@gmail.com>

commit 1babd96
Author: Marco Pracucci <marco@pracucci.com>
Date:   Thu Mar 19 09:21:37 2020 +0100

    Update integration/e2e/service.go

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

    Co-Authored-By: Peter Štibraný <pstibrany@gmail.com>

commit c1ed809
Author: Marco Pracucci <marco@pracucci.com>
Date:   Thu Mar 19 09:01:57 2020 +0100

    Added a timeout to the 'docker inspect' command to avoid the command being stuck

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

commit 271fb8d
Author: Marco Pracucci <marco@pracucci.com>
Date:   Thu Mar 19 08:52:09 2020 +0100

    Updated doc

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

commit a922d32
Author: Marco Pracucci <marco@pracucci.com>
Date:   Thu Mar 19 08:43:20 2020 +0100

    Fixed unit tests

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

commit 50398ab
Author: Marco Pracucci <marco@pracucci.com>
Date:   Thu Mar 19 08:40:40 2020 +0100

    Fixed doc

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

commit f077389
Author: Marco Pracucci <marco@pracucci.com>
Date:   Wed Mar 18 18:57:52 2020 +0100

    Documented blocks storage index cache backends

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

commit 330e449
Author: Marco Pracucci <marco@pracucci.com>
Date:   Wed Mar 18 18:14:12 2020 +0100

    Updated CHANGELOG

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

commit 0233519
Author: Marco Pracucci <marco@pracucci.com>
Date:   Thu Mar 12 17:40:21 2020 +0100

    Added memcached support to the blocks storage index cache

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

commit 305b16c
Author: Marco Pracucci <marco@pracucci.com>
Date:   Thu Mar 19 10:46:43 2020 +0100

    Introduced a basic logger to the integration tests framework (cortexproject#2293)

    * Introduced a basic logger to the integration tests framework

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

    * Fixed linter

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

    * Increased integration tests timeout in CI

    Signed-off-by: Marco Pracucci <marco@pracucci.com>

commit 51f7047
Author: Jay Batra <jaybatra73@gmail.com>
Date:   Thu Mar 19 13:19:39 2020 +0530

    Removes all the occurences of omitempty (cortexproject#2209)

    * Removes all the occurences of omitempty(cortexproject#2209)

    This PR removes all the occurences of omitempty
    from structs.

    Signed-off by Jay Batra <jaybatra73@gmail.com>

    Signed-off-by: Jay Batra <jaybatra73@gmail.com>

    * Update CHANGELOG.md

    Co-authored-by: Marco Pracucci <marco@pracucci.com>

commit bf59f9f
Author: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>
Date:   Wed Mar 18 17:00:11 2020 +0530

    support for cassandra in integration tests (cortexproject#2275)

    * support for cassandra in integration tests

    Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>

    * changes suggested from PR review

    Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>

    * using ConcreteService.Endpoint for getting http endpoint for readiness probe

    Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>

commit 87fa252
Author: Rajat Vig <rajatvig@users.noreply.github.com>
Date:   Wed Mar 18 08:49:45 2020 +0000

    Fix the import for test for bucket_client (cortexproject#2285)

    Signed-off-by: Rajat Vig <rvig@etsy.com>
@pracucci pracucci mentioned this pull request Mar 30, 2020
Closed
simonswine pushed a commit to grafana/e2e that referenced this pull request Jan 13, 2022
@sandeepsukhani
support for cassandra in integration tests (cortexproject/cortex#2275)
012d735 
* support for cassandra in integration tests

Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>

* changes suggested from PR review

Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>

* using ConcreteService.Endpoint for getting http endpoint for readiness probe

Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pstibrany pstibrany pstibrany approved these changes

@pracucci pracucci pracucci 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.
3 participants
@sandeepsukhani @pstibrany @pracucci