Sane Overridable Defaults

[katrogan]

Sane overridable defaults

Background

Flyte comes with a rich set of overridable defaults (matchable resources) from everything to default task resource requests, project resource quota, kubernetes labels and annotations for executions and default service accounts.

These can be configured to have default levels across a Flyte installation, on a per-domain basis, on a per-project basis and event at a per-workflow basis.

Managing these requires setting the defaults at a level of specifity using a variety of message types. Furthermore it's difficult to holistically reason about what overrides exist at say, the project level. Updating these overrides is error-prone because there is no existing merge logic and requires understanding the data model to even know what specific matchable resource to specifically update.

Proposal

Using kubectl apply as an inspiration, we should be able to make it simple to view, edit and update defaults for domains, projects and workflows. It should be difficult to unintentionally overwrite changes and easy to understand what is currently applied.

Overview

Proposed User Experience

It should be simple to view and edit overridable defaults

flytectl get overrides -o overrides.yaml
...

flytectl get overrides -d development -p flytesnacks -o overrides.yaml
...

For

• global/per-domain defaults (immutable application settings)
• project defaults
• per-workflow defaults

Proposed document structure

A single call to get all overrides should return the collated document for all overrides set at a level (e.g. per project)

$ flytectl get overrides -p flytesnacks -d development --workflow MyExampleWorkflow

apiVersion: 1  # Version of this document structure
domain: development
project: flytesnacks
workflow: MyExampleWorkflow
kind: overrides
spec:
  task_resources:
    defaults:
      cpu: "1"
      memory: "500Mi"
    limits:
      cpu: "3"
      memory: "2Gi"
      gpu: "4"
  workflow_execution_config:
    inherited: true
    source: project
    labels:
      my-key: "my-value"
    security_context:
      core_identity:
        iam_role: "my_iam_role"
status:
  version: 1 # Version for the entire overrides document

Note a few details

• The spec is the yaml formatted list of all available matchable resource messages defined
• Specific matchable resources can be inherited, that is, not set at the workflow level used to get the document. In this example, the workflow execution config is set at the project level
• The status includes a version. This is managed on the backend.

Implementation Proposal

Define a new set of APIs for returning overridable defaults

rpc GetOverrides(GetOverridesRequest) returns (GetOverridesResponse)

rpc UpdateOverrides(UpdateOverridesRequest) return (UpdateOverridesResponse)

...

message OverridesDocument {
  string domain = 1;
  
  string project = 2; // optional
  
  string workflow = 3; // optional
  
  int32 api_version = 4;
  
  repeated ResourceOverrides overrides = 5;
  
  OverridesStatus status = 6;
}

message ResourceOverrides {
  MatchingAttributes attributes = 1; # Oneof for existing matchable resources
  
  bool inherited = 2;
  
  string domain = 3;  // optional, if inherited
  
  string project = 4;  // optional, if inherited
  
  ...
}

message OverridesStatus {
  int32 version = 1;
}

FlyteAdmin changes

Update overrides

In implementing the endpoint handlers above, FlyteAdmin will need to create a new table to store document version for each scope of overrides (that is at the per project or per domain level).

model Overrides {
  Version int
  Project string
  Domain string
  Workflow string
}

So that the call to update overrides can only be applied when its version matches what is stored in the database. This prevents stale data from being committed and possibly overwriting a newer set of overrides that would need to be merged.

A call to update overrides should consist of a transaction to

1. Ensure the document passed in the call has a version which matches what exists in the overrides table
2. Update the overrides table with a new, monotonically increasing version
3. Update the matchable resources table with the desired set of overrides for anything not explicitly marked inherited in the document.
   i) To support override deletion, any overrides in the database which do not exist in the document should be removed.

Get Overrides

Getting overrides at a super specific level, for example at the workflow level, should use the same fallback logic at execution time to determine applicable overrides.

That is, the workflow-level document should include all applicable workflow overrides, any project overrides and any domain overrides in one document with inherited overrides marked as such.

Migration Strategy

Eventually we should store the entire document as part of the overrides table and move away from the confusing resources table. This allows us to do a finite number of gets at execution time to fetch the entire list of realized overrides for the global, domain, project and workflow level for all overridable attributes

To get there, we can create a new flyteadmin binary that runs a migration from the resources table and flattens it as an entry in the overrides table.

[kumare3]

@katrogan thank you for this proposal. This is an amazing RFC and will really simply the user lives. +1

[honnix]

Love it!

A quick question regarding update. What do you think we make it closer to how kubectl works, either client-side or server-side apply, instead of having our own way relying on inherited?

[fg91]

I think it is a great idea to simplify this!

We do have project/domain specific pod templates but don't override further defaults. You said that doing so is error-prone, difficult to holistically reason about, and requires understanding about the data model. I couldn't have put it into these words but I would say that when I did look at this part of the documentation in the past, "it didn't look inviting to try this unless one really has to".

---

One other important reason, apart from complexity/UX, why I'd be a bit hesitant to use this feature in case one of our platform users suggested to have defaults overridden for their project, is that we try to stay away from any platform config (Flyte and in general) that we cannot put into declarative infrastructure as code that we can version control. As I'm reluctant to use kubectl in an imperative way (create, edit, ...) for a prod environment, I'd be reluctant to configure our ML platform in an imperative way using flytectl.
Our Flyte deployment, including projects, defaults, ... is 100% deployable and reproducible from scratch with 2 terraform apply commands (one for the GKE cluster, database, .... and one for everything in the cluster). Other users might use GitOps tools like ArgoCD or FluxCD.

I'm aware that in some situations where the documentation tells the user to use flytectl in an imperative way, e.g. to modify execution cluster labels in a multi-cluster setup, there is an equivalent configuration that one can set in the helm values/flyteadmin configmap. I don't know whether the same applies to the overridable defaults but I think it would be really great if the outcome of this effort would be that these options can be configured declaratively in addition to improving the UX when doing so with flytectl and that this is documented. Do you think this is possible/makes sense as part of this RFC? Or should this be a separate one?

[honnix]

I second the need of declarative infra, as it is also a concrete problem we have been dealing with.

I see a lot things are happening in flytectl as proposed by this RFC, which is a pure client thing. If we could make this happening on the server side, for example by introducing a k8s operator taking in custom resource (kind: overrides) reconciling, and making change to flyteadmin, we will probably don't need to extend flyteadmin and don't need to invent our own way of version tracking either.

[honnix]

I might be relating too much to our ways of working (https://engineering.atspotify.com/2023/05/fleet-management-at-spotify-part-2-the-path-to-declarative-infrastructure/). So just food for thought.

[fg91]

I might be relating too much to our ways of working (https://engineering.atspotify.com/2023/05/fleet-management-at-spotify-part-2-the-path-to-declarative-infrastructure/). So just food for thought.

Very cool! Creating custom internal services built on top of managed cloud services (like the kind: Locus showcased in the blog post) that developers can consume as K8s CRDs is pretty much exactly what is described here as a good/the best way to implement an internal developer platform (idp).

While I really(!) like the approach, I wonder whether we can keep it a bit simpler here: flyteadmin is currently configured via configmaps which are parameterized in the helm values. If we created a new configmap/new section in an existing configmap for these overrides, we wouldn't need a new operator. This is also the approach currently in place for modifying execution cluster labels in a multi-cluster setup mentioned above.

One downside is that pods currently don't restart when configmaps change, see here. However, this might change in the future and there appear to be workarounds - if really required.

[katrogan]

+100 to the idea of managing this through IaC, thank you both for the excellent suggestions! Revised the proposal in #3749 to move towards this instead.

@fg91 agreed that a ConfigMap could be a sane compromise for bootstrapping this off the ground and keeping the configuration entirely declarative

@honnix I really like the idea of using K8s operators for managing desired config state. I think that's something we should absolutely move towards but possibly as a dedicated follow-up that is the second big piece of this work. Once we've aligned on a ConfigMap/document structure then we should be able extend how the config is managed

[davidmirror-ops]

@katrogan so I think considering the feedback from the community, feel free to kickoff the process for a new RFC!
Any question let us know

[katrogan]

@davidmirror-ops thanks, opened #3749

[fg91]

Closing this as the real RFC #3749 has already been accepted.