Skip to content

Branched environment is never added if its first snapshot build fails #6591

Description

@markphelps

Problem

A branch pushed directly to Git under an environment's branch prefix (for example flipt/staging/fix) is discovered on the next poll. If the branch's first snapshot build fails (for example because of invalid YAML in default/features.yaml), the branched environment never reaches the environment store until the process restarts. Fixing the branch afterwards doesn't bring it back.

Cause

In RefreshEnvironment (store.go:700-731):

  1. The new branch environment is cached in e.branches and appended to the local NewBranches before env.updateSnapshot runs.
  2. updateSnapshot fails, and the function returns (nil, err). NewBranches is discarded, so the subscriber in NewStore never calls envStore.Add (environments.go:579-590). updateSubs only logs the error.
  3. e.refs[branch] is never set on this path, so the next poll fails again on the same branch.
  4. From the third poll on, the branch is treated as already known and is skipped. That path never adds to NewBranches, including after a fix commit.

Side effects:

  • While those two polls fail, the branches after the bad one are not discovered or updated, and deleted branches are not pruned.
  • The cached environment keeps a snapshot of the base branch, because NewEnvironmentFromRepo builds its first snapshot from the default branch (store.go:97-111), and it reports HasSnapshot() == true.

Branches created through the API are not affected, because EnvironmentStore.Branch adds them directly.

Reproduce

This test fails: polls 1-2 return the YAML error, polls 3-4 succeed, and the branch is cached but never delivered.

test added to internal/storage/environments/git/store_test.go
func Test_RefreshEnvironment_BranchFirstSnapshotFailure(t *testing.T) {
	env := newTestEnvironment(t, "production")
	ctx := t.Context()

	const badBranch = "flipt/production/bad"
	require.NoError(t, env.repo.CreateBranchIfNotExists(ctx, badBranch, storagegit.WithBase(env.currentBranch)))
	_, err := env.repo.UpdateAndPush(ctx, badBranch, func(f fs.Filesystem) (string, error) {
		if err := f.MkdirAll("default", 0o755); err != nil {
			return "", err
		}
		fi, err := f.OpenFile("default/features.yaml", os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0o644)
		if err != nil {
			return "", err
		}
		if _, err := fi.Write([]byte("namespace: [unterminated\nflags: {{{\n")); err != nil {
			return "", err
		}
		return "bad yaml", fi.Close()
	})
	require.NoError(t, err)

	currentRefs := func() map[string]string {
		references, err := env.repo.References()
		require.NoError(t, err)
		refs := map[string]string{}
		_ = references.ForEach(func(r *plumbing.Reference) error {
			if r.Name().IsRemote() {
				refs[strings.TrimPrefix(r.Name().String(), "refs/remotes/origin/")] = r.Hash().String()
			}
			return nil
		})
		return refs
	}

	// simulate the store's notifyFn: only envs in NewBranches reach the EnvironmentStore
	added := map[string]bool{}
	for poll := 1; poll <= 4; poll++ {
		result, err := env.RefreshEnvironment(ctx, currentRefs())
		t.Logf("poll %d: err=%v", poll, err)
		if err != nil {
			continue
		}
		for _, b := range result.NewBranches {
			added[b.Key()] = true
		}
	}

	env.mu.RLock()
	_, cached := env.branches["bad"]
	env.mu.RUnlock()
	assert.True(t, cached, "bad branch env cached in e.branches")
	assert.True(t, added["bad"], "bad branch env was never delivered via NewBranches to the EnvironmentStore")
}

Suggested fix

  • Only cache a new branch in e.branches (and record its ref) after its snapshot builds successfully.
  • Don't let one failing branch abort the loop: collect the errors, keep processing the other branches and pruning, and return the combined error at the end.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    • Status
      No status
    • Status
      No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions