jetstack / navigator

Managed Database-as-a-Service (DBaaS) on Kubernetes
Apache License 2.0
271 stars 31 forks source link

Add 'version' field to ESC. Auto-generate Elasticsearch image details #205

Closed munnerz closed 6 years ago

munnerz commented 6 years ago

What this PR does / why we need it:

This PR adds a version field to an ElasticsearchCluster. It is used to auto-default the ElasticsearchImageSpec in its absence. If a user wants to override the image to be used and take manual control, they can specify their own fields as before.

The new version field should be required regardless whether the image is manually specified, as in future it can be used to help influence upgrade strategy. It will be up to the administrator to update the image and listed version in unison.

In future we should make the default image repository and uid be configurable.

Special notes for your reviewer:

Ensuring the spec.version field is set will be in a follow up after #199 is merged

Release note:

Add a new spec.version field to ElasticsearchCluster and auto-default the image version and repository to an official upstream image.
munnerz commented 6 years ago

/test e2e

munnerz commented 6 years ago

/test e2e

jetstack-ci-bot commented 6 years ago

@munnerz PR needs rebase

munnerz commented 6 years ago

/test e2e

munnerz commented 6 years ago

/test e2e

munnerz commented 6 years ago

@wallrj I've updated the PR to make the new Version field in our API into a semver.Version.

The unit tests obviously work with Golang structs (and pass), but they don't test the unmarshalling from a string. I've added a version parameter to the Elasticsearch Cluster test fixture for our e2e tests in order to verify this works correctly, and from what I can see it is passing happily.

Once #199 merges, I'll add further validation to this to stop invalid values being submitted to the API.

munnerz commented 6 years ago

I've added additional validation to this now, which ensures that version is specified.

Unmarshaling will error if an invalid string is provided, but it doesn't error if no string is provided (it returns nil). In order to handle this case, I check for an empty semver.Version struct during validation and throw a Required error in this case.

jetstack-ci-bot commented 6 years ago

@munnerz PR needs rebase

jetstack-ci-bot commented 6 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wallrj

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

Needs approval from an approver in each of these OWNERS Files: - ~~[OWNERS](https://github.com/jetstack/navigator/blob/master/OWNERS)~~ [wallrj] 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 commented 6 years ago

@munnerz PR needs rebase

jetstack-ci-bot commented 6 years ago

/lgtm cancel //PR changed after LGTM, removing LGTM. @munnerz @wallrj

jetstack-ci-bot commented 6 years ago

/lgtm cancel //PR changed after LGTM, removing LGTM. @munnerz @wallrj

jetstack-ci-bot commented 6 years ago

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

jetstack-ci-bot commented 6 years ago

Automatic merge from submit-queue.