cockroachdb/cockroach: pkg/migration/manager.go; 96 LoC

Found a possible issue in [cockroachdb/cockroach](https://www.github.com/cockroachdb/cockroach) at [pkg/migration/manager.go](https://github.com/cockroachdb/cockroach/blob/4e3a9ccdb9cac89ae32d6248bb9b119b5bc250f7/pkg/migration/manager.go#L101-L196)

Below is the message reported by the analyzer for this snippet of code. Beware that the analyzer only reports the first
issue it finds, so please do not limit your consideration to the contents of the below message.

> reference to clusterVersion was used in a composite literal at line 161

[Click here to see the code in its original context.](https://github.com/cockroachdb/cockroach/blob/4e3a9ccdb9cac89ae32d6248bb9b119b5bc250f7/pkg/migration/manager.go#L101-L196)

Click here to show the 96 line(s) of Go which triggered the analyzer.

```go
for _, clusterVersion := range clusterVersions {
cluster := newCluster(m.nl, m.dialer, m.executor, m.db)
h := newHelper(cluster, clusterVersion)

// Push out the version gate to every node in the cluster. Each node
// will persist the version, bump the local version gates, and then
// return. The migration associated with the specific version can assume
// that every node in the cluster has the corresponding version
// activated.
//
// We'll need to first bump the fence version for each intermediate
// cluster version, before bumping the "real" one. Doing so allows us to
// provide the invariant that whenever a cluster version is active, all
// nodes in the cluster (including ones added concurrently during
// version upgrades) are running binaries that know about the version.

{
// The migrations infrastructure makes use of internal fence
// versions when stepping through consecutive versions. It's
// instructive to walk through how we expect a version migration
// from v21.1 to v21.2 to take place, and how we behave in the
// presence of new v21.1 or v21.2 nodes being added to the cluster.
// - All nodes are running v21.1
// - All nodes are rolled into v21.2 binaries, but with active
// cluster version still as v21.1
// - The first version bump will be into v21.2-1(fence), see the
// migration manager above for where that happens
// Then concurrently:
// - A new node is added to the cluster, but running binary v21.1
// - We try bumping the cluster gates to v21.2-1(fence)
//
// If the v21.1 nodes manages to sneak in before the version bump,
// it's fine as the version bump is a no-op one (all fence versions
// are). Any subsequent bumps (including the "actual" one bumping to
// v21.2) will fail during the validation step where we'll first
// check to see that all nodes are running v21.2 binaries.
//
// If the v21.1 node is only added after v21.2-1(fence) is active,
// it won't be able to actually join the cluster (it'll be prevented
// by the join RPC).
//
// All of which is to say that once we've seen the node list
// stabilize (as EveryNode enforces), any new nodes that can join
// the cluster will run a release that support the fence version,
// and by design also supports the actual version (which is the
// direct successor of the fence).
fenceVersion := fenceVersionFor(ctx, clusterVersion)
req := &serverpb.BumpClusterVersionRequest{ClusterVersion: &fenceVersion}
op := fmt.Sprintf("bump-cluster-version=%s", req.ClusterVersion.PrettyPrint())
err := h.EveryNode(ctx, op, func(ctx context.Context, client serverpb.MigrationClient) error {
_, err := client.BumpClusterVersion(ctx, req)
return err
})
if err != nil {
return err
}
}
{
// Now sanity check that we'll actually be able to perform the real
// cluster version bump, cluster-wide.
req := &serverpb.ValidateTargetClusterVersionRequest{ClusterVersion: &clusterVersion}
op := fmt.Sprintf("validate-cluster-version=%s", req.ClusterVersion.PrettyPrint())
err := h.EveryNode(ctx, op, func(ctx context.Context, client serverpb.MigrationClient) error {
_, err := client.ValidateTargetClusterVersion(ctx, req)
return err
})
if err != nil {
return err
}
}
{
// Finally, bump the real version cluster-wide.
req := &serverpb.BumpClusterVersionRequest{ClusterVersion: &clusterVersion}
op := fmt.Sprintf("bump-cluster-version=%s", req.ClusterVersion.PrettyPrint())
if err := h.EveryNode(ctx, op, func(ctx context.Context, client serverpb.MigrationClient) error {
_, err := client.BumpClusterVersion(ctx, req)
return err
}); err != nil {
return err
}
}

// TODO(irfansharif): We'll want to be able to override which migration
// is retrieved here within tests. We could make the registry be a part
// of the manager, and all tests to provide their own.

// Finally, run the actual migration.
migration, ok := registry[clusterVersion]
if !ok {
log.Infof(ctx, "no migration registered for %s, skipping", clusterVersion)
continue
}
if err := migration.Run(ctx, h); err != nil {
return err
}
}

```

Leave a reaction on this issue to contribute to the project by classifying this instance as a **Bug** :-1:, **Mitigated** :+1:, or **Desirable Behavior** :rocket:
See the descriptions of the classifications [here](https://github.com/github-vet/rangeclosure-findings#how-can-i-help) for more information.

commit ID: 4e3a9ccdb9cac89ae32d6248bb9b119b5bc250f7

Read pkg/migration/manager.go around lines 101-196 and inspect the analyzer's range-loop reference to clusterVersion in the composite literals. Determine whether this is a bug, mitigated behavior, or desirable behavior, then leave the corresponding reaction classification on the issue.

Written by the indexing model from the issue text.