hashgraph / hashgraph/solo-weaver

ops label profile: decouple instance_type from --cluster-name and environment from --profile

Open
#1,087 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
3
Forks
0
Avg merge
3d 5h
Merged PRs (30d)
47

Description

### Feature to Enhance

The `ops` label profile in `alloy cluster install` (added in #847), specifically how it sources the `instance_type` and `environment` labels.

Both are derived from inputs that already carry another responsibility:

- **`instance_type`** = the alphabetic prefix of the first dash-separated segment of `--cluster-name` (`internal/alloy/labels/ops.go`, `ParseClusterName` / `extractAlphaPrefix`). It is written in exactly one place and has no explicit input.
- **`environment`** = `LabelInput.DeployProfile`, i.e. whatever `--profile` resolved to, which is validated against `models.SupportedProfiles()` = `[mainnet testnet previewnet perfnet local]`.

So the cluster name doubles as a node-role declaration, and the deployment profile doubles as an environment declaration. Neither coupling can be worked around from the caller side without giving up control of the other value.

### Requested Enhancement

1. **Take the node role (`instance_type`) as an explicit input** rather than parsing it out of `--cluster-name`.

Weaver already models this distinction internally — `PresetTier1LFH = "tier1-lfh"` / `PresetTier1RFH = "tier1-rfh"` (`internal/blocknode/blocknode_plugins.go:24-27`, consumed by `pkg/hardware/provider_block.go` via `presetPredicate("tier1-rfh")`), plus `NodeTypeBlock` / `NodeTypeConsensus` (`pkg/models/node_types.go:8-9`). The information exists; it just doesn't reach the label layer.

An explicit input would also let the emitted value follow whatever taxonomy the deploying organization already uses for `instance_type` — many operators set that label explicitly at the collector for their other node classes, with values like `-block-node`, and would want block nodes to join that scheme rather than inherit a value that falls out of a hostname string.

2. **Allow `environment` to be set independently of the deployment profile.** Any of these unblocks the use case, in descending order of preference:
- an explicit environment/label override flag on `alloy cluster install`;
- accepting arbitrary `--profile` values, bringing the flag to parity with the config-file path (see Additional Context);
- deriving `environment` from segment 2 of the cluster name when the deploy profile is empty or `local`. `ParseClusterName` already splits on the documented `--` convention and then discards segment 2, so this is the smallest change — but it re-entrenches the cluster-name coupling that item 1 is trying to remove.

3. **Make validation consistent** across whichever inputs survive.

### Business Need

**Any deployment whose environment name is not a member of `SupportedProfiles()` cannot get a correct `environment` label.** `SupportedProfiles()` is a closed set of five values, but real fleets run environments outside it — development, staging, integration, QA, per-team or per-tenant environments. Those deployments must pass `--profile local`, and every one of them then reports `environment="local"`, indistinguishable from each other and from an actual local install.

The label appears to work only when an environment's name happens to coincide with a profile name. That is a coincidence, not configuration: there is no input that expresses "this is environment X" independently of "provision using hardware/profile Y".

By contrast, a collector-based pipeline typically sets `environment` as a literal resource attribute, so it is correct for every environment by construction. The label profile is the only place in such a stack where the value is inferred rather than declared.

**`instance_type` silently produces a wrong value whenever the cluster name is not role-first.** The derivation takes the first segment, so:

| `--cluster-name` | resulting `instance_type` |
|---|---|
| `host01-env-a` | `host` ✅ (role-first) |
| `env-a-host01` | `env` ❌ |
| `--` | `` ❌ |

Cluster names are frequently chosen for human legibility, or by third-party operators following a naming scheme handed to them, and nothing at the point of naming surfaces the constraint that segment 1 must be the node role. The result is a plausible name that yields a label which is wrong but well-formed, so nothing errors.

**This failure mode silently disables alerting.** An alert selecting `node_filesystem_avail_bytes{instance_type=~"lfh|rfh"}` matches nothing at all on a cluster whose name is not role-first. There is no error, no missing-metric warning, and no indication in the label set that the value was derived rather than declared — the alert simply evaluates against an empty set indefinitely. Discovering this requires someone to notice the absence.

### Additional Context

**The flag and config-file paths disagree about validation.** `alloy cluster install` validates the profile only when it arrives as a flag:

- `cmd/cli/commands/alloy/cluster/install.go` — `if flagProfile != "" { if !hardware.IsValidProfile(flagProfile) { … } ; config.SetProfile(flagProfile) }`
- when the flag is omitted the profile comes from the `--config` file, and `pkg/config.Initialize` performs no profile validation; `Config.Validate()` (`pkg/models/config.go`) checks `labelProfile` on each remote but never `Profile`; `hardware.IsValidProfile` is never called on a config-loaded value.
- the install steps then consume it verbatim: `alloy.NewConfigBuilder(cfg, config.Get().Profile)` (`internal/workflows/steps/step_alloy.go:144`, `:473`, `:567`, `:659`).

So a config file containing `profile: ` already yields `environment=""`, while the equivalent `--profile=` is rejected. That looks like an oversight rather than an intended interface, so it isn't something to depend on — but it does show the label layer does not actually require profile values to be enum members, which makes item 2 cheaper than it first appears.

**Workaround in use, and why it isn't sufficient.** Cluster names can be forced role-first everywhere, which makes `instance_type` resolve correctly. That works, but it makes a string convention load-bearing across an entire fleet, enforced only by convention and invisible at the point where names are chosen — the brittleness this request is asking to remove. It also does nothing for `environment`, which has no caller-side workaround at all.

Related: #847 (added `--profile` and the ops-profile instance label).

Contributor guide

Open the contributing guide

Research direction

Start with internal/alloy/labels/ops.go, especially ParseClusterName and extractAlphaPrefix, then trace the install flag handling in cmd/cli/commands/alloy/cluster/install.go and profile use in internal/workflows/steps/step_alloy.go. Compare config validation in pkg/config and pkg/models/config.go with hardware.IsValidProfile; done means explicit role and environment inputs work independently and validation is consistent across flag and config paths.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, cli
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.