Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Use the Docker maintained Cassandra 3 image #229

Merged
merged 1 commit into from
Jan 31, 2018

Conversation

wallrj
Copy link
Member

@wallrj wallrj commented Jan 31, 2018

  • Use the Docker maintained Cassandra 3 image.
  • Use TCP connect based readiness and liveness probe because this image doesn't contain a nodetool based /ready-probe.sh script.
  • These TCP connection checks will succeed even though the process is stopped; so I now simulate a node failure using nodetool decommission which makes cassandra stop listening on its CQL port.
  • I can improve on this when Jolokia based readiness and liveness probes #170 is merged.

Fixes: #222

Release note:

NONE

@wallrj wallrj changed the title WIP: Use the Docker maintained Cassandra 3 image Use the Docker maintained Cassandra 3 image Jan 31, 2018
* Use the Docker maintained Cassandra 3 image.
* Temporarily use TCP connect based readiness and liveness probes because this image doesn't contain a nodetool based `/ready-probe.sh` script.
* I found that these TCP connection checks succeed even when the process is SIGSTOPped; so I now simulate a node failure using `nodetool decommission` which makes cassandra stop listening on its CQL port.
* Cluster status aware readiness probes will be restored jetstack#170 is merged.

Fixes: jetstack#222
tag: {{ .Values.image.tag }}
pullPolicy: {{ .Values.image.pullPolicy }}
repository: {{ .Values.image.repository | quote }}
tag: {{ .Values.image.tag | quote }}
Copy link
Member Author

Choose a reason for hiding this comment

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

I got helm lint errors when I used an unquoted integer here.

https://github.com/kubernetes/helm/blob/master/docs/charts_tips_and_tricks.md#quote-strings-dont-quote-integers suggests quoting all strings, so I decided to do that.

{
Name: "CASSANDRA_AUTO_BOOTSTRAP",
Value: "false",
},
Copy link
Member Author

Choose a reason for hiding this comment

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

The Docker image doesn't pay attention to this environment variable so I removed it.

We'll see if it causes any dataloss problems when I rebase #225

Copy link
Contributor

@munnerz munnerz left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

// The /run.sh script is unique to gcr.io/google-samples/cassandra:v12.
// TODO: Add support for other Cassandra images with different entry points.
cmd := exec.Command("/run.sh")
cmd := exec.Command("/docker-entrypoint.sh")
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we’ll still have issues handling SIGTERM with this? Unless they have properly set up signal handling in this script.

@jetstack-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: munnerz

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@jetstack-ci-bot
Copy link
Contributor

/test all [submit-queue is verifying that this PR is safe to merge]

@jetstack-ci-bot
Copy link
Contributor

Automatic merge from submit-queue.

@jetstack-ci-bot jetstack-ci-bot merged commit efaac39 into jetstack:master Jan 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants