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.

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

helm: add support for PMM #3598

Merged
merged 4 commits into from
Feb 2, 2018
Merged

helm: add support for PMM #3598

merged 4 commits into from
Feb 2, 2018

Conversation

derekperkins
Copy link
Member

This adds optional support to auto-deploy PMM as a separate StatefulSet in Kubernetes, plus adds a pmm-client container to vttablet, along with 2 log tailing containers, which auto-register with the server. It enables mysql metrics and query analytics by default.

The PMM client requires root privileges in the container, so I was forced to remove runAsNonRoot from the default Pod Security Context, but all other containers are still running as a non-root user, so it shouldn't be an issue.

@derekperkins
Copy link
Member Author

Also of note, I'm ignoring client errors until https://jira.percona.com/projects/PMM/issues/PMM-1985 is resolved. The client works, and worst case scenario where we ignore an error, it won't have any impact on actual Vitess/MySQL operations.

@enisoc This is ready for review

# Copy CA certs for https calls
COPY --from=base /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt

RUN apt-get update && \
Copy link
Member

Choose a reason for hiding this comment

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

We should put all these RUN commands into script file, and just COPY and RUN that. Currently since these are separate layers, we're still paying for the storage of all the removed files.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I can do that

apt-get upgrade -qq && \
apt-get install wget -qq --no-install-recommends

RUN wget https://www.percona.com/redir/downloads/pmm-client/1.6.1/binary/debian/stretch/x86_64/pmm-client_1.6.1-1.stretch_amd64.deb
Copy link
Member

Choose a reason for hiding this comment

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

Do they not have an apt repo we can add?

Copy link
Member Author

Choose a reason for hiding this comment

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

They do. This seemed easier, but I can switch to their apt setup

# --force is used because the pod ip address may have changed
pmm-admin config --server pmm.{{ $namespace }} --force

# each of these creates a systemd service
Copy link
Member

Choose a reason for hiding this comment

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

It seems weird to install a systemd service in a container. Can we not just run the server directly?

Copy link
Member Author

Choose a reason for hiding this comment

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

Their whole setup is very opinionated and there isn't a way around this. You have to use their pmm-admin client and it doesn't have an option to run another way.

@derekperkins
Copy link
Member Author

I batched all the RUN commands together and it brought the image size from 72 MB down to 45 MB, so that was a win. Is there a reason using the apt repo is better than just grabbing the binary?

@enisoc
Copy link
Member

enisoc commented Feb 2, 2018

LGTM

Approved with PullApprove

@enisoc enisoc merged commit 5aef3c1 into vitessio:master Feb 2, 2018
@derekperkins derekperkins deleted the pmm branch March 2, 2018 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants