Skip to content

Rework the instance_name config api #1649

@aaronmondal

Description

@aaronmondal

Somewhat of a follow-up to #834.

Our current configuration looks like this:

    "services": {
      "cas": {
        "main": {
          "cas_store": "WORKER_FAST_SLOW_STORE"
        }
      },
      "ac": {
        "main": {
          "ac_store": "AC_MAIN_STORE"
        }
      },
      "execution": {
        "main": {
          "cas_store": "WORKER_FAST_SLOW_STORE",
          "scheduler": "MAIN_SCHEDULER"
        }
      },
      "capabilities": {
        "main": {
          "remote_execution": {
            "scheduler": "MAIN_SCHEDULER"
          }
        }
      },
      "bytestream": {
        "cas_stores": {
          "main": "WORKER_FAST_SLOW_STORE"
        }
      }

The "main" here refers to the "instance_name" as per https://github.com/bazelbuild/remote-apis/blob/2721568dea746d81a98a116ee222c8da50bbcf5c/build/bazel/remote/execution/v2/remote_execution.proto#L1405.

There are multiple issues with this approach:

  • This has repeatedly been a source of confusion for users as it's not immediately clear that this is the instance name without consulting additional documentation.
  • It's a dynamic key, which is incompatible with how K8s wants CRDs to look like. In other words, you can't create correct yaml with this.
  • The default should be no instance name. This looks ugly as it requires an empty string key:
"cas": {
  "": {
    "cas_store": "WORKER_FAST_SLOW_STORE"
  }
},

The initially mentioned discussion was about a similar issue. In the same spirit, it seems better to use something along these lines:

    "services": {
      "cas": {
        "instance_name": "main",
        "cas_store": "WORKER_FAST_SLOW_STORE"
      },
      "ac": {
        "instance_name": "main",
        "ac_store": "AC_MAIN_STORE"
      },
      "execution": {
        "instance_name": "main",
        "cas_store": "WORKER_FAST_SLOW_STORE",
        "scheduler": "MAIN_SCHEDULER"
      },
      "capabilities": {
        "instance_name": "main",
        "remote_execution": {
          "scheduler": "MAIN_SCHEDULER"
        }
      },
      "bytestream": {
        "cas_stores": [
        {
          "instance_name": "main",
          "ref": "WORKER_FAST_SLOW_STORE"
        }]
      }

This solves all above mentioned issues because

  • It's self-documenting
  • We're no longer using a dynamic key for the instance name.
  • If we make the "instance_name" field optional an default to an empty string if not set users can omit it entirely and we get this:
"cas": {
  "cas_store": "WORKER_FAST_SLOW_STORE"
},

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions