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

exit server if not all Ark CRDs exist at startup #683

Merged
merged 1 commit into from
Jul 19, 2018

Conversation

skriss
Copy link
Contributor

@skriss skriss commented Jul 18, 2018

Signed-off-by: Steve Kriss steve@heptio.com

@skriss skriss added the Bug label Jul 18, 2018
@skriss skriss added this to the v0.9.1 milestone Jul 18, 2018
@skriss skriss requested review from carlisia and nrb July 18, 2018 20:21
@skriss skriss force-pushed the fail-fast-if-missing-crds branch from 16950a0 to 8d31472 Compare July 18, 2018 20:31

// CustomResources returns a map of all custom resources within the Ark
// API group, keyed on Kind.
func CustomResources() map[string]typeInfo {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to re-use https://github.com/heptio/ark/blob/master/pkg/install/crd.go here? That way we'd have them defined in one place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather go the other way around (have pkg/install use the pkg/apis/ark/v1 func), but yeah good point, I'll update the install package.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that direction does make more sense. If we use one common source then we're less likely to miss requiring any new CRDs getting added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

@skriss skriss force-pushed the fail-fast-if-missing-crds branch from 8d31472 to ad0f94a Compare July 18, 2018 21:05
@skriss
Copy link
Contributor Author

skriss commented Jul 18, 2018

Working on adding a unit test for the server func.

Signed-off-by: Steve Kriss <steve@heptio.com>
@skriss skriss force-pushed the fail-fast-if-missing-crds branch from ad0f94a to 1df9a8a Compare July 18, 2018 21:30
@skriss
Copy link
Contributor Author

skriss commented Jul 18, 2018

added unit.

@skriss
Copy link
Contributor Author

skriss commented Jul 19, 2018

Manual testing looks good on this - if any CRD is missing, the server exits. If all are present, the server proceeds.

@carlisia
Copy link
Contributor

I 👀 👍

@nrb nrb merged commit 7a964ae into vmware-tanzu:master Jul 19, 2018
@skriss skriss deleted the fail-fast-if-missing-crds branch July 20, 2018 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants