-
Notifications
You must be signed in to change notification settings - Fork 40
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
Add custom installation volume support #1092
Conversation
Kubernetes volumes can now be added to installation deployments. Volumes backed by secrets can be created, updated, and deleted after an installation has been created. The volume secret data is first stored in AWS Secrets Manager and then appended as a k8s secret in an installation update. Volumes are mounted in the specified mount path of the container which should support many cases where sensitive information is required at the filesystem level of Mattermost.
b863210
to
2c1f9bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small callout but overall LGTM. Nicely done!
|
||
oldState := installationDTO.State | ||
|
||
err = createVolumeRequest.Apply(installationDTO.Installation) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to validate anything with this? Even if as simple as checking for conflicts with current volume names?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, we definitely want to do one final validation to look for issues. If you trace out the Apply
you will end up here were we run some final checks: https://github.com/mattermost/mattermost-cloud/pull/1092/files#diff-888e481ee6df2e28a4dbed85a127b1d24d0a6d7cbefbdda42792d926dc1ec70bR63
command.Flags().StringVar(&flags.installationID, "installation", "", "The id of the installation to create a volume for.") | ||
command.Flags().StringVar(&flags.volumeName, "volume-name", "", "The name of the volume to create.") | ||
command.Flags().StringVar(&flags.mountPath, "mount-path", "", "The container path to mount the volume in.") | ||
command.Flags().BoolVar(&flags.readOnly, "read-only", true, "Whether the volume should be read only or not.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a read-only flag as a mount of type Secret is always read-only https://kubernetes.io/docs/concepts/storage/volumes/#secret
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, but this is just prepping for the future when we may have other volume types.
Kubernetes volumes can now be added to installation deployments. Volumes backed by secrets can be created, updated, and deleted after an installation has been created. The volume secret data is first stored in AWS Secrets Manager and then appended as a k8s secret in an installation update. Volumes are mounted in the specified mount path of the container which should support many cases where sensitive information is required at the filesystem level of Mattermost.
Fixes https://mattermost.atlassian.net/browse/CLD-8711