Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,11 @@
templateID api.TemplateID,
body api.TemplateBuildRequest,
) (*template.RegisterBuildResponse, *api.APIError) {
// TODO(ENG-3852): Stop sending the firecracker version from the API. The orchestrator
// now resolves its own FC version via the FirecrackerVersions feature flag
// (see packages/orchestrator/pkg/template/build/builder.go)
// TODO(ENG-3852): Stop sending the firecracker/kernel versions from the API.
// The orchestrator resolves them itself via the BuildFirecrackerVersion /
// BuildKernelVersion feature flags (see packages/orchestrator/pkg/template/server/create_template.go).
firecrackerVersion := a.featureFlags.StringFlag(ctx, featureflags.BuildFirecrackerVersion)
kernelVersion := a.featureFlags.StringFlag(ctx, featureflags.BuildKernelVersion)

var alias *string
var tags []string
Expand Down Expand Up @@ -206,7 +207,7 @@
CpuCount: body.CpuCount,
MemoryMB: body.MemoryMB,
Version: templates.TemplateV1Version,
KernelVersion: a.config.DefaultKernelVersion,
KernelVersion: kernelVersion,

Check warning on line 210 in packages/api/internal/handlers/deprecated_template_request_build.go

View check run for this annotation

Claude / Claude Code Review

Dead code: Config.DefaultKernelVersion no longer referenced

Nit: this PR removes the only consumers of `Config.DefaultKernelVersion` (both `a.config.DefaultKernelVersion` reads in this file and in `template_request_build_v3.go` are replaced with the `BuildKernelVersion` feature flag). That leaves the `DefaultKernelVersion` field in `packages/api/internal/cfg/model.go` (line 71, already marked `// Deprecated`) and its default-fill block (lines 141-142) as dead code — worth a follow-up cleanup, not blocking this PR.
Comment thread
ValentaTomas marked this conversation as resolved.
FirecrackerVersion: firecrackerVersion,
}

Expand Down
9 changes: 5 additions & 4 deletions packages/api/internal/handlers/template_request_build_v3.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,11 @@ func requestTemplateBuild(ctx context.Context, c *gin.Context, a *APIStore, body
}
span.End()

// TODO(ENG-3852): Stop sending the firecracker version from the API. The orchestrator
// now resolves its own FC version via the FirecrackerVersions feature flag
// (see packages/orchestrator/pkg/template/build/builder.go)
// TODO(ENG-3852): Stop sending the firecracker/kernel versions from the API.
// The orchestrator resolves them itself via the BuildFirecrackerVersion /
// BuildKernelVersion feature flags (see packages/orchestrator/pkg/template/server/create_template.go).
firecrackerVersion := a.featureFlags.StringFlag(ctx, featureflags.BuildFirecrackerVersion)
kernelVersion := a.featureFlags.StringFlag(ctx, featureflags.BuildKernelVersion)
buildReq := template.RegisterBuildData{
ClusterID: clusters.WithClusterFallback(team.ClusterID),
TemplateID: templateID,
Expand All @@ -129,7 +130,7 @@ func requestTemplateBuild(ctx context.Context, c *gin.Context, a *APIStore, body
CpuCount: body.CpuCount,
MemoryMB: body.MemoryMB,
Version: templates.TemplateV2LatestVersion,
KernelVersion: a.config.DefaultKernelVersion,
KernelVersion: kernelVersion,
FirecrackerVersion: firecrackerVersion,
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (s *ServerStore) TemplateCreate(ctx context.Context, templateRequest *templ
featureflags.TeamContext(cfg.GetTeamID()),
)

kernelVersion := featureflags.DefaultKernelVersion
kernelVersion := s.featureFlags.StringFlag(ctx, featureflags.BuildKernelVersion)
firecrackerVersion := s.featureFlags.StringFlag(ctx, featureflags.BuildFirecrackerVersion)

fcInfo, err := fcversion.New(firecrackerVersion)
Expand Down
7 changes: 6 additions & 1 deletion packages/shared/pkg/featureflags/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,12 @@ func (c *Client) IntFlag(ctx context.Context, flag IntFlag, contexts ...ldcontex
}

func (c *Client) StringFlag(ctx context.Context, flag StringFlag, contexts ...ldcontext.Context) string {
return getFlag(ctx, c.ld, c.ld.StringVariationCtx, flag, c.allContexts(contexts))
v := getFlag(ctx, c.ld, c.ld.StringVariationCtx, flag, c.allContexts(contexts))
if v == "" && flag.FallbackWhenEmpty() {
return flag.Fallback()
}

return v
}

type typedFlag[T any] interface {
Expand Down
22 changes: 19 additions & 3 deletions packages/shared/pkg/featureflags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,9 @@ var (
)

type StringFlag struct {
name string
fallback string
name string
fallback string
fallbackWhenEmpty bool
}

func (f StringFlag) Key() string {
Expand All @@ -230,6 +231,12 @@ func (f StringFlag) Fallback() string {
return f.fallback
}

// FallbackWhenEmpty reports whether the fallback should also be used when LD
// returns an empty string (e.g. flag defined in LD without a value).
func (f StringFlag) FallbackWhenEmpty() bool {
return f.fallbackWhenEmpty
}

func NewStringFlag(name string, fallback string) StringFlag {
flag := StringFlag{name: name, fallback: fallback}
builder := launchDarklyOfflineStore.Flag(flag.name).ValueForAll(ldvalue.String(fallback))
Expand All @@ -238,7 +245,15 @@ func NewStringFlag(name string, fallback string) StringFlag {
return flag
}

// This is currently not configurable via feature flags.
// NewStringFlagFallbackOnEmpty is like NewStringFlag but also returns the
// fallback when LD evaluates the flag to an empty string.
func NewStringFlagFallbackOnEmpty(name string, fallback string) StringFlag {
flag := NewStringFlag(name, fallback)
flag.fallbackWhenEmpty = true

return flag
}

const (
DefaultKernelVersion = "vmlinux-6.1.158"
)
Expand All @@ -261,6 +276,7 @@ var FirecrackerVersionMap = map[string]string{
// BuildIoEngine Sync is used by default as there seems to be a bad interaction between Async and a lot of io operations.
var (
BuildFirecrackerVersion = NewStringFlag("build-firecracker-version", env.GetEnv("DEFAULT_FIRECRACKER_VERSION", DefaultFirecrackerVersion))
BuildKernelVersion = NewStringFlagFallbackOnEmpty("build-kernel-version", env.GetEnv("DEFAULT_KERNEL_VERSION", DefaultKernelVersion))
Comment thread
ValentaTomas marked this conversation as resolved.
Outdated
BuildIoEngine = NewStringFlag("build-io-engine", "Sync")
DefaultPersistentVolumeType = NewStringFlag("default-persistent-volume-type", "")
BuildNodeInfo = NewJSONFlag("preferred-build-node", ldvalue.Null())
Expand Down
Loading