Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: Internal implementation of the "refresh only" planning mode and "replace" planning option #28560

Merged
merged 3 commits into from
Apr 30, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
11 changes: 6 additions & 5 deletions backend/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,12 @@ type Operation struct {

// The options below are more self-explanatory and affect the runtime
// behavior of the operation.
PlanMode plans.Mode
AutoApprove bool
Parallelism int
Targets []addrs.Targetable
Variables map[string]UnparsedVariableValue
PlanMode plans.Mode
AutoApprove bool
Parallelism int
Targets []addrs.Targetable
ForceReplace []addrs.AbsResourceInstance
Variables map[string]UnparsedVariableValue

// Some operations use root module variables only opportunistically or
// don't need them at all. If this flag is set, the backend must treat
Expand Down
2 changes: 2 additions & 0 deletions backend/local/backend_local.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ func (b *Local) context(op *backend.Operation) (*terraform.Context, *configload.
// Copy set options from the operation
opts.PlanMode = op.PlanMode
opts.Targets = op.Targets
opts.ForceReplace = op.ForceReplace
opts.UIInput = op.UIIn
opts.Hooks = op.Hooks

Expand Down Expand Up @@ -264,6 +265,7 @@ func (b *Local) contextFromPlanFile(pf *planfile.Reader, opts terraform.ContextO
opts.Variables = variables
opts.Changes = plan.Changes
opts.Targets = plan.TargetAddrs
opts.ForceReplace = plan.ForceReplaceAddrs
opts.ProviderSHA256s = plan.ProviderSHA256s

tfCtx, ctxDiags := terraform.NewContext(&opts)
Expand Down
443 changes: 264 additions & 179 deletions plans/internal/planproto/planfile.pb.go

Large diffs are not rendered by default.

20 changes: 20 additions & 0 deletions plans/internal/planproto/planfile.proto
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ message Plan {
// the file for another message- or field-specific reason.
uint64 version = 1;

// The mode that was active when this plan was created.
//
// This is saved only for UI purposes, so that Terraform can tailor its
// rendering of the plan depending on the mode. This must never be used to
// make decisions in Terraform Core during the applying of a plan.
Mode mode = 17;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about changing the name of this to really make the intent obvious? The comment is great, and I'm happy to call it sufficient, but I wouldn't mind renaming the field ModeForDisplay or something that's as descriptive as some of our functions that only print for-display-purposes-only strings (literally ForDisplay())

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh! If we foresee other similar fields in the future, I wouldn't be opposed to a plans.DisplayInfo (or whatever name makes more sense) struct that contains this information, and those other fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point, and the only thing that gives me pause is that we usually use "for display" to mean something than be directly displayed to the user, rather than an indicator used as part of choosing what to display. But the string representations of these modes are not user-friendly, at least as currently designed.

I'm thinking about ui_mode as an alternative. I think it isn't quite right but hopefully at least sufficient to prompt a potential user of it to notice the prefix is there and wonder what it means enough to come read this comment. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r.e. similar fields: I hesitate to generalize this too soon cause we only have two examples of this so far and they've been in different parts of the structure anyway. Since our plan file format isn't something we promise as stable between releases we will be free to reshape this later if we have new requirements that suggest a different design.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, I'm down with ui_mode - anything that makes the scope of that field unavoidably obvious to a developer (me!) is awesome.
I also agree (not that you said this directly) that my second suggestion is a premature optimization, and we can see what's actually useful in time.


// The variables that were set when creating the plan. Each value is
// a msgpack serialization of an HCL value.
map<string, DynamicValue> variables = 2;
Expand All @@ -37,6 +44,12 @@ message Plan {
// configuration.
repeated string target_addrs = 5;

// An unordered set of force-replace addresses to include when applying.
// This must match the set of addresses that was used when creating the
// plan, or else applying the plan will fail when it reaches a different
// conclusion about what action a particular resource instance needs.
repeated string force_replace_addrs = 16;

// The version string for the Terraform binary that created this plan.
string terraform_version = 14;

Expand All @@ -49,6 +62,13 @@ message Plan {
Backend backend = 13;
}

// Mode describes the planning mode that created the plan.
enum Mode {
NORMAL = 0;
DESTROY = 1;
REFRESH_ONLY = 2;
}

// Backend is a description of backend configuration and other related settings.
message Backend {
string type = 1;
Expand Down
21 changes: 15 additions & 6 deletions plans/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,21 @@ import (
// since the plan does not itself include all of the information required to
// make the changes indicated.
type Plan struct {
VariableValues map[string]DynamicValue
Changes *Changes
TargetAddrs []addrs.Targetable
ProviderSHA256s map[string][]byte
Backend Backend
State *states.State
// Mode is the mode under which this plan was created.
//
// This is only recorded to allow for UI differences when presenting plans
// to the end-user, and so it must not be used to influence apply-time
// behavior. The actions during apply must be described entirely by
// the Changes field, regardless of how the plan was created.
Mode Mode

VariableValues map[string]DynamicValue
Changes *Changes
TargetAddrs []addrs.Targetable
ForceReplaceAddrs []addrs.AbsResourceInstance
ProviderSHA256s map[string][]byte
Backend Backend
State *states.State
}

// Backend represents the backend-related configuration and other data as it
Expand Down
34 changes: 34 additions & 0 deletions plans/planfile/tfplan.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,17 @@ func readTfplan(r io.Reader) (*plans.Plan, error) {
ProviderSHA256s: map[string][]byte{},
}

switch rawPlan.Mode {
case planproto.Mode_NORMAL:
plan.Mode = plans.NormalMode
case planproto.Mode_DESTROY:
plan.Mode = plans.DestroyMode
case planproto.Mode_REFRESH_ONLY:
plan.Mode = plans.RefreshOnlyMode
default:
return nil, fmt.Errorf("plan has invalid mode %s", rawPlan.Mode)
}

for _, rawOC := range rawPlan.OutputChanges {
name := rawOC.Name
change, err := changeFromTfplan(rawOC.Change)
Expand Down Expand Up @@ -95,6 +106,14 @@ func readTfplan(r io.Reader) (*plans.Plan, error) {
plan.TargetAddrs = append(plan.TargetAddrs, target.Subject)
}

for _, rawReplaceAddr := range rawPlan.ForceReplaceAddrs {
addr, diags := addrs.ParseAbsResourceInstanceStr(rawReplaceAddr)
if diags.HasErrors() {
return nil, fmt.Errorf("plan contains invalid force-replace address %q: %s", addr, diags.Err())
}
plan.ForceReplaceAddrs = append(plan.ForceReplaceAddrs, addr)
}

for name, rawHashObj := range rawPlan.ProviderHashes {
if len(rawHashObj.Sha256) == 0 {
return nil, fmt.Errorf("no SHA256 hash for provider %q plugin", name)
Expand Down Expand Up @@ -343,6 +362,17 @@ func writeTfplan(plan *plans.Plan, w io.Writer) error {
ResourceChanges: []*planproto.ResourceInstanceChange{},
}

switch plan.Mode {
case plans.NormalMode:
rawPlan.Mode = planproto.Mode_NORMAL
case plans.DestroyMode:
rawPlan.Mode = planproto.Mode_DESTROY
case plans.RefreshOnlyMode:
rawPlan.Mode = planproto.Mode_REFRESH_ONLY
default:
return fmt.Errorf("plan has unsupported mode %s", plan.Mode)
}

for _, oc := range plan.Changes.Outputs {
// When serializing a plan we only retain the root outputs, since
// changes to these are externally-visible side effects (e.g. via
Expand Down Expand Up @@ -380,6 +410,10 @@ func writeTfplan(plan *plans.Plan, w io.Writer) error {
rawPlan.TargetAddrs = append(rawPlan.TargetAddrs, targetAddr.String())
}

for _, replaceAddr := range plan.ForceReplaceAddrs {
rawPlan.ForceReplaceAddrs = append(rawPlan.ForceReplaceAddrs, replaceAddr.String())
}

for name, hash := range plan.ProviderSHA256s {
rawPlan.ProviderHashes[name] = &planproto.Hash{
Sha256: hash,
Expand Down
136 changes: 113 additions & 23 deletions terraform/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,15 @@ const (
// ContextOpts are the user-configurable options to create a context with
// NewContext.
type ContextOpts struct {
Config *configs.Config
Changes *plans.Changes
State *states.State
Targets []addrs.Targetable
Variables InputValues
Meta *ContextMeta
PlanMode plans.Mode
SkipRefresh bool
Config *configs.Config
Changes *plans.Changes
State *states.State
Targets []addrs.Targetable
ForceReplace []addrs.AbsResourceInstance
Variables InputValues
Meta *ContextMeta
PlanMode plans.Mode
SkipRefresh bool

Hooks []Hook
Parallelism int
Expand Down Expand Up @@ -100,6 +101,7 @@ type Context struct {
refreshState *states.State
skipRefresh bool
targets []addrs.Targetable
forceReplace []addrs.AbsResourceInstance
variables InputValues
meta *ContextMeta
planMode plans.Mode
Expand Down Expand Up @@ -256,6 +258,17 @@ func NewContext(opts *ContextOpts) (*Context, tfdiags.Diagnostics) {
switch opts.PlanMode {
case plans.NormalMode, plans.DestroyMode:
// OK
case plans.RefreshOnlyMode:
if opts.SkipRefresh {
// The CLI layer (and other similar callers) should prevent this
// combination of options.
diags = diags.Append(tfdiags.Sourceless(
tfdiags.Error,
"Incompatible plan options",
"Cannot skip refreshing in refresh-only mode. This is a bug in Terraform.",
))
return nil, diags
}
default:
// The CLI layer (and other similar callers) should not try to
// create a context for a mode that Terraform Core doesn't support.
Expand All @@ -266,6 +279,16 @@ func NewContext(opts *ContextOpts) (*Context, tfdiags.Diagnostics) {
))
return nil, diags
}
if len(opts.ForceReplace) > 0 && opts.PlanMode != plans.NormalMode {
// The other modes don't generate no-op or update actions that we might
// upgrade to be "replace", so doesn't make sense to combine those.
diags = diags.Append(tfdiags.Sourceless(
tfdiags.Error,
"Unsupported plan mode",
fmt.Sprintf("Forcing resource instance replacement (with -replace=...) is allowed only in normal planning mode."),
))
return nil, diags
}

log.Printf("[TRACE] terraform.NewContext: complete")

Expand Down Expand Up @@ -293,6 +316,7 @@ func NewContext(opts *ContextOpts) (*Context, tfdiags.Diagnostics) {
refreshState: state.DeepCopy(),
skipRefresh: opts.SkipRefresh,
targets: opts.Targets,
forceReplace: opts.ForceReplace,
uiInput: opts.UIInput,
variables: variables,

Expand Down Expand Up @@ -327,13 +351,14 @@ func (c *Context) Graph(typ GraphType, opts *ContextGraphOpts) (*Graph, tfdiags.
switch typ {
case GraphTypeApply:
return (&ApplyGraphBuilder{
Config: c.config,
Changes: c.changes,
State: c.state,
Components: c.components,
Schemas: c.schemas,
Targets: c.targets,
Validate: opts.Validate,
Config: c.config,
Changes: c.changes,
State: c.state,
Components: c.components,
Schemas: c.schemas,
Targets: c.targets,
ForceReplace: c.forceReplace,
Validate: opts.Validate,
}).Build(addrs.RootModuleInstance)

case GraphTypeValidate:
Expand All @@ -351,13 +376,14 @@ func (c *Context) Graph(typ GraphType, opts *ContextGraphOpts) (*Graph, tfdiags.
case GraphTypePlan:
// Create the plan graph builder
return (&PlanGraphBuilder{
Config: c.config,
State: c.state,
Components: c.components,
Schemas: c.schemas,
Targets: c.targets,
Validate: opts.Validate,
skipRefresh: c.skipRefresh,
Config: c.config,
State: c.state,
Components: c.components,
Schemas: c.schemas,
Targets: c.targets,
ForceReplace: c.forceReplace,
Validate: opts.Validate,
skipRefresh: c.skipRefresh,
}).Build(addrs.RootModuleInstance)

case GraphTypePlanDestroy:
Expand All @@ -370,6 +396,20 @@ func (c *Context) Graph(typ GraphType, opts *ContextGraphOpts) (*Graph, tfdiags.
Validate: opts.Validate,
}).Build(addrs.RootModuleInstance)

case GraphTypePlanRefreshOnly:
// Create the plan graph builder, with skipPlanChanges set to
// activate the "refresh only" mode.
return (&PlanGraphBuilder{
Config: c.config,
State: c.state,
Components: c.components,
Schemas: c.schemas,
Targets: c.targets,
Validate: opts.Validate,
skipRefresh: c.skipRefresh,
skipPlanChanges: true, // this activates "refresh only" mode.
}).Build(addrs.RootModuleInstance)

case GraphTypeEval:
return (&EvalGraphBuilder{
Config: c.config,
Expand Down Expand Up @@ -551,6 +591,8 @@ The -target option is not for routine use, and is provided only for exceptional
plan, planDiags = c.plan()
case plans.DestroyMode:
plan, planDiags = c.destroyPlan()
case plans.RefreshOnlyMode:
plan, planDiags = c.refreshOnlyPlan()
default:
panic(fmt.Sprintf("unsupported plan mode %s", c.planMode))
}
Expand Down Expand Up @@ -603,7 +645,9 @@ func (c *Context) plan() (*plans.Plan, tfdiags.Diagnostics) {
return nil, diags
}
plan := &plans.Plan{
Changes: c.changes,
Mode: plans.NormalMode,
Changes: c.changes,
ForceReplaceAddrs: c.forceReplace,
}

c.refreshState.SyncWrapper().RemovePlannedResourceInstanceObjects()
Expand Down Expand Up @@ -656,10 +700,56 @@ func (c *Context) destroyPlan() (*plans.Plan, tfdiags.Diagnostics) {
return nil, diags
}

destroyPlan.Mode = plans.DestroyMode
destroyPlan.Changes = c.changes
return destroyPlan, diags
}

func (c *Context) refreshOnlyPlan() (*plans.Plan, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics

graph, graphDiags := c.Graph(GraphTypePlanRefreshOnly, nil)
diags = diags.Append(graphDiags)
if graphDiags.HasErrors() {
return nil, diags
}

// Do the walk
walker, walkDiags := c.walk(graph, walkPlan)
diags = diags.Append(walker.NonFatalDiagnostics)
diags = diags.Append(walkDiags)
if walkDiags.HasErrors() {
return nil, diags
}
plan := &plans.Plan{
Mode: plans.RefreshOnlyMode,
Changes: c.changes,
}

// If the graph builder and graph nodes correctly obeyed our directive
// to refresh only, the set of resource changes should always be empty.
// We'll safety-check that here so we can return a clear message about it,
// rather than probably just generating confusing output at the UI layer.
if len(plan.Changes.Resources) != 0 {
diags = diags.Append(tfdiags.Sourceless(
tfdiags.Error,
"Invalid refresh-only plan",
"Terraform generated planned resource changes in a refresh-only plan. This is a bug in Terraform.",
))
}

c.refreshState.SyncWrapper().RemovePlannedResourceInstanceObjects()

refreshedState := c.refreshState.DeepCopy()
plan.State = refreshedState

// replace the working state with the updated state, so that immediate calls
// to Apply work as expected.
c.state = refreshedState

return plan, diags
}

// Refresh goes through all the resources in the state and refreshes them
// to their latest state. This is done by executing a plan, and retaining the
// state while discarding the change set.
Expand Down
Loading