First pass at shared validation #533
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dturn
force-pushed
the
consistent-validations
branch
from
1024e61
to
6477c39
Compare
KnVerey
reviewed
Aug 22, 2019
dturn
force-pushed
the
consistent-validations
branch
2 times, most recently
from
ce62469
to
6edce21
Compare
dturn
force-pushed
the
consistent-validations
branch
from
3ee3e37
to
74a8d62
Compare
timothysmith0609
approved these changes
Aug 29, 2019
dturn
force-pushed
the
consistent-validations
branch
from
7487af2
to
641d7a6
Compare
RyanBrushett
approved these changes
Aug 30, 2019
Merged
ghost
added
the
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are you trying to accomplish with this PR?
Consistent preflight validations for all tasks. Inspiration for this PR taken from https://github.com/Shopify/kubernetes-deploy/compare/config_validator
I've started by just adding this to restart task. If we like where this is going I can add it to the remaining tasks in this PR or subsequent ones.
How is this accomplished?
Our run/restart/deploy tasks have overlapping state they need, I've attempted to extract it into TaskConfig object. For now this is context, namespace, and logger.
I've also added methods for generating(We decided against generating kubectl, kubeclient_builder in task_config, see the comments for the discussion about why). When we go tokubectl, andkubeclient_builder, its possible that this is backwards and those methods should take a task_config.KraneI think the TaskConfig should be passed to the task instead of it building it. (Though maybe not since that would require users using this as a library to construct a 'random' object, perhaps two constructors for each task).I've also created a Validator object that can validate, kubeconfig, context, namespace and kubectl version. This looks to be the shared set of validations. Each task has additional validations, this PR leaves open several approaches to solving that problem.
What could go wrong?
Because we need to unify validations that are done slightly differently we are changing the error class that gets raised as well as the wording. If people were rescuing these errors or matching text this will be a breaking change.