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

TRON-2195: Support watcher_kubeconfig_paths #989

Merged
merged 5 commits into from
Jul 11, 2024

Conversation

jfongatyelp
Copy link
Contributor

Requires Yelp/task_processing#213 & new version of taskproc to be released.

This adds the support to tron's MASTER config's k8s_options to allow the new old_kubeconfig_paths option and passes it along to task_processing's kubernetes_pod_executor.

@cuza
Copy link
Member

cuza commented Jul 10, 2024

don't we need to add validation and a default value for this in tron/config/config_parse.py? Something like:

class ValidateKubernetes(Validator):
    config_class = ConfigKubernetes
    optional = True
    defaults = {
        "kubeconfig_path": None,
        "old_kubeconfig_paths": None
        "enabled": False,
        "default_volumes": (),
    }

    validators = {
        "kubeconfig_path": valid_string,
        "old_kubeconfig_paths": valid_string
        "enabled": valid_bool,
        "default_volumes": build_list_of_type_validator(valid_volume, allow_empty=True),
    }

@jfongatyelp
Copy link
Contributor Author

jfongatyelp commented Jul 10, 2024

don't we need to add validation and a default value for this in tron/config/config_parse.py?

Thanks! That's probably a good call to maintain parity w/ all the rest of our configs + validate values for watcher_kubeconfig_paths. That said I've always been a bit confused why we specify defaults both in config_parse and in the schema classes themselves. Currently this still works fine w/o errors (verified on one of our test tron servers w/ setting + unsetting watcher_kubeconfig_paths and updating via paasta_setup_tron_namespace) since we are just using the ConfigKubernetes schema class (which I've updated in this PR) in config_parse, and so it gets the default I've specified there.

@jfongatyelp jfongatyelp force-pushed the jfong/TRON-2195-old-kubeconfig-paths branch from dff06e1 to 48a5b4a Compare July 10, 2024 21:29
@jfongatyelp jfongatyelp changed the title TRON-2195: Support old_kubeconfig_paths TRON-2195: Support watcher_kubeconfig_paths Jul 10, 2024
@jfongatyelp jfongatyelp merged commit 95ad61a into master Jul 11, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants