Update paasta validate to use eks instance type where necessary - PAASTA-17992 #3683
Conversation
EmanElsaban
force-pushed
the
u/emanelsabban/PAASTA-17992
branch
from
2d9eb35
to
5a877fb
Compare
paasta_tools/cli/cmds/validate.py
Outdated
| if file_type == "eks": | ||
| # since eks_schema.json is a symlink to kubernetes_schema.json | ||
| file_type = "kubernetes" |
There was a problem hiding this comment.
we should be able to let the symlink do the redirection here (i.e., not update file_type and let this code read schemas/eks_schema.json)
There was a problem hiding this comment.
ohh, I think I linked the eks_schema.json to the full path instead of just "kubernetes_schema.json" and thus wasn't able to open the file. Will correct that
paasta_tools/cli/cmds/validate.py
Outdated
| config_file_object, file_path | ||
| ): | ||
| if ( | ||
| file_type == "kubernetes" or file_type == "eks" |
There was a problem hiding this comment.
we do this check of if X == "kubernetes" or X == "eks" in several places - thoughts on adding a named constant like K8S_TYPES = {"eks, "kubernetes"}?
we could then use it like if file_type in K8S_TYPES or if instance_config.get_instance_type() in K8S_TYPES
paasta_tools/cli/cmds/validate.py
Outdated
| workload="eks" | ||
| if instance_config.get_instance_type() == "eks" | ||
| else "kubernetes", |
There was a problem hiding this comment.
workload=instance_config.get_instance_type() should do the same, right? if i'm following this correctly, we should only get into this block of code if the instance type is eks or kubernetes, so this should do the same thing :)
paasta_tools/cli/cmds/validate.py
Outdated
| f"eks-{cluster}.yaml" | ||
| if instance_config.get_instance_type() == "eks" | ||
| else f"kubernetes-{cluster}.yaml", |
There was a problem hiding this comment.
somewhat the same as above: would f"{instance_config.get_instance_type()}-{cluster}.yaml" do the same thing?
This PR checks for EKS instance types wherever we check for Kubernetes instance types. Since eks_schema.json is a symbolic link to kubernetes_schema.json, I had to override in
def get_schema(file_type)the eks file_type since eks_schema.json doesn't actually exist so we get noneTesting