Skip to content
This repository has been archived by the owner on Nov 15, 2022. It is now read-only.

check for duplicate volumeClaim definition #204

Merged
merged 1 commit into from
Aug 8, 2017

Conversation

surajssd
Copy link
Member

@surajssd surajssd commented Aug 8, 2017

If a volumeClaim with same name is defined now the checks are done,
to fail on such a case.

Fixes #55

@surajssd surajssd requested review from kadel, cdrage and concaf August 8, 2017 05:32
Copy link
Member

@kadel kadel left a comment

Choose a reason for hiding this comment

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

Other than one small comment regarding test LGTM


func TestValidateVolumeClaims(t *testing.T) {

failingTests := [][]spec.VolumeClaim{
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to have also one passing test

Copy link
Member Author

Choose a reason for hiding this comment

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

@kadel done

If a volumeClaim with same name is defined now the checks are done,
to fail on such a case.
@surajssd surajssd force-pushed the multiple-pvc-same-name branch from b39e217 to 5de239c Compare August 8, 2017 08:01
@surajssd
Copy link
Member Author

surajssd commented Aug 8, 2017

@kadel I have done the update as you asked so PTAL! :-)

@kedge-bot
Copy link
Collaborator

@surajssd, thank you for the pull request! We'll ping some people to review your PR. @containscafeine, please review this.

Copy link
Collaborator

@cdrage cdrage left a comment

Choose a reason for hiding this comment

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

LGTM here too

@surajssd surajssd merged commit dbcd73c into kedgeproject:master Aug 8, 2017
@surajssd
Copy link
Member Author

surajssd commented Aug 8, 2017

@cdrage @kadel thanks!

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.

4 participants