Skip to content

Commit

Permalink
bugfix: runtime apply and detail prompt cancel unexpectly
Browse files Browse the repository at this point in the history
- runtime apply should differ create or patch, json merge patch cannot applied to an object which is not existed
- prompt cycle has a cancel condition, which is triggered every time
- refactor computing node's action logic
- use live state and plan state to make a diff report which is consistent with runtime logic
  • Loading branch information
howieyuen committed Jun 7, 2022
1 parent d88c533 commit d4d7245
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 102 deletions.
2 changes: 1 addition & 1 deletion pkg/engine/operation/change.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type ChangeStep struct {
// TODO: 3-way diff
func (cs *ChangeStep) Diff() (string, error) {
// Generate diff report
diffReport, err := diffToReport(cs.Original, cs.Modified)
diffReport, err := diffToReport(cs.Current, cs.Modified)
if err != nil {
log.Errorf("failed to compute diff with ChangeStep ID: %s", cs.ID)
return "", err
Expand Down
10 changes: 5 additions & 5 deletions pkg/engine/operation/preview.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type PreviewResponse struct {
Order *ChangeOrder
}

func (o *Operation) Preview(request *PreviewRequest, operation Type) (rsp *PreviewResponse, s status.Status) {
func (o *Operation) Preview(request *PreviewRequest) (rsp *PreviewResponse, s status.Status) {
defer func() {
if e := recover(); e != nil {
log.Error("preview panic:%v", e)
Expand Down Expand Up @@ -56,11 +56,11 @@ func (o *Operation) Preview(request *PreviewRequest, operation Type) (rsp *Previ
// 1. init & build Indexes
priorState, resultState = initStates(o.StateStorage, &request.Request)

switch operation {
case Apply:
switch o.OperationType {
case ApplyPreview:
priorStateResourceIndex = priorState.Resources.Index()
graph, s = NewApplyGraph(request.Manifest, priorState)
case Destroy:
case DestroyPreview:
resources := request.Request.Manifest.Resources
priorStateResourceIndex = resources.Index()
graph, s = NewDestroyGraph(resources)
Expand All @@ -74,7 +74,7 @@ func (o *Operation) Preview(request *PreviewRequest, operation Type) (rsp *Previ

previewOperation := &PreviewOperation{
Operation: Operation{
OperationType: Preview,
OperationType: o.OperationType,
StateStorage: o.StateStorage,
CtxResourceIndex: map[string]*models.Resource{},
PriorStateResourceIndex: priorStateResourceIndex,
Expand Down
61 changes: 34 additions & 27 deletions pkg/engine/operation/preview_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ var (
ID: "fake-id",
Attributes: FakeService,
}
FakeResourceState2 = models.Resource{
ID: "fake-id-2",
Attributes: FakeService,
}
)

var _ runtime.Runtime = (*fakePreviewRuntime)(nil)
Expand All @@ -40,7 +44,11 @@ func (f *fakePreviewRuntime) Apply(ctx context.Context, priorState, planState *m
}

func (f *fakePreviewRuntime) Read(ctx context.Context, resourceState *models.Resource) (*models.Resource, status.Status) {
return resourceState, nil
if resourceState.ResourceKey() == "fake-id" {
return nil, nil
} else {
return resourceState, nil
}
}

func (f *fakePreviewRuntime) Delete(ctx context.Context, resourceState *models.Resource) status.Status {
Expand All @@ -65,8 +73,7 @@ func TestOperation_Preview(t *testing.T) {
lock *sync.Mutex
}
type args struct {
request *PreviewRequest
operation Type
request *PreviewRequest
}
tests := []struct {
name string
Expand All @@ -78,9 +85,10 @@ func TestOperation_Preview(t *testing.T) {
{
name: "success-when-apply",
fields: fields{
Runtime: &fakePreviewRuntime{},
StateStorage: &states.FileSystemState{Path: states.KusionState},
Order: &ChangeOrder{StepKeys: []string{}, ChangeSteps: map[string]*ChangeStep{}},
OperationType: ApplyPreview,
Runtime: &fakePreviewRuntime{},
StateStorage: &states.FileSystemState{Path: states.KusionState},
Order: &ChangeOrder{StepKeys: []string{}, ChangeSteps: map[string]*ChangeStep{}},
},
args: args{
request: &PreviewRequest{
Expand All @@ -96,7 +104,6 @@ func TestOperation_Preview(t *testing.T) {
},
},
},
operation: Apply,
},
wantRsp: &PreviewResponse{
Order: &ChangeOrder{
Expand All @@ -117,9 +124,10 @@ func TestOperation_Preview(t *testing.T) {
{
name: "success-when-destroy",
fields: fields{
Runtime: &fakePreviewRuntime{},
StateStorage: &states.FileSystemState{Path: states.KusionState},
Order: &ChangeOrder{},
OperationType: DestroyPreview,
Runtime: &fakePreviewRuntime{},
StateStorage: &states.FileSystemState{Path: states.KusionState},
Order: &ChangeOrder{},
},
args: args{
request: &PreviewRequest{
Expand All @@ -130,23 +138,22 @@ func TestOperation_Preview(t *testing.T) {
Operator: "fake-operator",
Manifest: &models.Spec{
Resources: []models.Resource{
FakeResourceState,
FakeResourceState2,
},
},
},
},
operation: Destroy,
},
wantRsp: &PreviewResponse{
Order: &ChangeOrder{
StepKeys: []string{"fake-id"},
StepKeys: []string{"fake-id-2"},
ChangeSteps: map[string]*ChangeStep{
"fake-id": {
ID: "fake-id",
"fake-id-2": {
ID: "fake-id-2",
Action: Delete,
Original: &FakeResourceState,
Modified: (*models.Resource)(nil),
Current: &FakeResourceState,
Original: &FakeResourceState2,
Modified: &FakeResourceState2,
Current: &FakeResourceState2,
},
},
},
Expand All @@ -156,27 +163,28 @@ func TestOperation_Preview(t *testing.T) {
{
name: "fail-because-empty-models",
fields: fields{
Runtime: &fakePreviewRuntime{},
StateStorage: &states.FileSystemState{Path: states.KusionState},
Order: &ChangeOrder{},
OperationType: ApplyPreview,
Runtime: &fakePreviewRuntime{},
StateStorage: &states.FileSystemState{Path: states.KusionState},
Order: &ChangeOrder{},
},
args: args{
request: &PreviewRequest{
Request: Request{
Manifest: nil,
},
},
operation: Apply,
},
wantRsp: nil,
wantS: status.NewErrorStatusWithMsg(status.InvalidArgument, "request.Spec is empty. If you want to delete all resources, please use command 'destroy'"),
},
{
name: "fail-because-nonexistent-id",
fields: fields{
Runtime: &fakePreviewRuntime{},
StateStorage: &states.FileSystemState{Path: states.KusionState},
Order: &ChangeOrder{},
OperationType: ApplyPreview,
Runtime: &fakePreviewRuntime{},
StateStorage: &states.FileSystemState{Path: states.KusionState},
Order: &ChangeOrder{},
},
args: args{
request: &PreviewRequest{
Expand All @@ -197,7 +205,6 @@ func TestOperation_Preview(t *testing.T) {
},
},
},
operation: Apply,
},
wantRsp: nil,
wantS: status.NewErrorStatusWithMsg(status.IllegalManifest, "can't find resource by key:nonexistent-id in models or state."),
Expand All @@ -217,7 +224,7 @@ func TestOperation_Preview(t *testing.T) {
resultState: tt.fields.resultState,
lock: tt.fields.lock,
}
gotRsp, gotS := o.Preview(tt.args.request, tt.args.operation)
gotRsp, gotS := o.Preview(tt.args.request)
if !reflect.DeepEqual(gotRsp, tt.wantRsp) {
t.Errorf("Operation.Preview() gotRsp = %v, want %v", kdump.FormatN(gotRsp), kdump.FormatN(tt.wantRsp))
}
Expand Down
78 changes: 43 additions & 35 deletions pkg/engine/operation/resource_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,59 +23,67 @@ var _ ExecutableNode = (*ResourceNode)(nil)

func (rn *ResourceNode) Execute(operation *Operation) status.Status {
log.Debugf("execute node:%s", rn.ID)
// 1. prepare planedState
planedState := rn.state
if rn.Action != Delete {
if operation.OperationType != Preview {
// replace implicit references
value := reflect.ValueOf(rn.state.Attributes)
_, implicitValue, s := ParseImplicitRef(value, operation.CtxResourceIndex, ImplicitReplaceFun)
if status.IsErr(s) {
return s
}
rn.state.Attributes = implicitValue.Interface().(map[string]interface{})
if operation.OperationType == Apply {
// replace implicit references
value := reflect.ValueOf(rn.state.Attributes)
_, implicitValue, s := ParseImplicitRef(value, operation.CtxResourceIndex, ImplicitReplaceFun)
if status.IsErr(s) {
return s
}
} else {
planedState = nil
rn.state.Attributes = implicitValue.Interface().(map[string]interface{})
}

// 1. prepare planedState
planedState := rn.state

// 2. get prior state which is stored in kusion_state.json
key := rn.state.ResourceKey()
priorState := operation.PriorStateResourceIndex[key]

// get the latest resource from runtime
liveState, s := operation.Runtime.Read(context.Background(), priorState)
// 3. get the latest resource from runtime
liveState, s := operation.Runtime.Read(context.Background(), planedState)
if status.IsErr(s) {
return s
}

// 3. compute ActionType of current resource node between planState and liveState
if liveState == nil {
rn.Action = Create
} else if planedState == nil {
// 4. compute ActionType of current resource node between planState and liveState
switch operation.OperationType {
case Destroy, DestroyPreview:
rn.Action = Delete
} else if reflect.DeepEqual(liveState, planedState) {
rn.Action = UnChange
} else {
rn.Action = Update
case Apply, ApplyPreview:
if liveState == nil {
rn.Action = Create
} else if planedState == nil {
rn.Action = Delete
} else if reflect.DeepEqual(liveState, planedState) {
rn.Action = UnChange
} else {
rn.Action = Update
}
default:
return status.NewErrorStatus(fmt.Errorf("unknown operation: %v", operation.OperationType))
}

if operation.OperationType == Preview {
// 5. apply or return
switch operation.OperationType {
case ApplyPreview, DestroyPreview:
fillResponseChangeSteps(operation, rn, priorState, planedState, liveState)
return nil
}
// 4. apply
switch rn.Action {
case Create, Delete, Update:
s := rn.applyResource(operation, priorState, planedState)
if status.IsErr(s) {
return s
case Apply, Destroy:
switch rn.Action {
case Create, Delete, Update:
s := rn.applyResource(operation, priorState, planedState)
if status.IsErr(s) {
return s
}
case UnChange:
log.Infof("PriorAttributes and PlanAttributes are equal.")
default:
return status.NewErrorStatus(fmt.Errorf("unknown action:%s", rn.Action.PrettyString()))
}
case UnChange:
log.Infof("PriorAttributes and PlanAttributes are equal.")
default:
return status.NewErrorStatus(fmt.Errorf("unknown op:%s", rn.Action.PrettyString()))
return status.NewErrorStatus(fmt.Errorf("unknown operation: %v", operation.OperationType))
}

return nil
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/engine/operation/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ type Type int64
const (
UndefinedOperation Type = iota // invalidate value
Apply
Preview
ApplyPreview
Destroy
DestroyPreview
)
56 changes: 34 additions & 22 deletions pkg/engine/runtime/kubernetest_runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"kusionstack.io/kusion/pkg/engine/models"
"kusionstack.io/kusion/pkg/log"
"kusionstack.io/kusion/pkg/status"
"kusionstack.io/kusion/pkg/util/json"
"kusionstack.io/kusion/pkg/util/kube/config"
"kusionstack.io/kusion/pkg/util/yaml"
)
Expand Down Expand Up @@ -57,34 +58,41 @@ func (k *KubernetesRuntime) Apply(ctx context.Context, priorState, planState *mo
if err != nil {
return nil, status.NewErrorStatus(err)
}

// original equals to last-applied from annotation, kusion store it in kusion_state.json
original := yaml.MergeToOneYAML(priorState.Attributes)
// modified equals input content
modified := yaml.MergeToOneYAML(planState.Attributes)
// get live state
liveState, err := resource.Get(ctx, planObj.GetName(), metav1.GetOptions{})
if err != nil {
return nil, status.NewErrorStatus(err)
liveState, s := k.Read(ctx, planState)
if status.IsErr(s) {
return nil, s
}
// current equals live manifest
current := yaml.MergeToOneYAML(liveState.Object)
// 3-way json merge patch
patch, err := jsonmergepatch.CreateThreeWayJSONMergePatch([]byte(original), []byte(modified), []byte(current))
if err != nil {
return nil, status.NewErrorStatus(err)
}
// apply patch
patchedObj, err := resource.Patch(ctx, planObj.GetName(), types.MergePatchType, patch, metav1.PatchOptions{
FieldManager: "kusion",
})
if err != nil {
return nil, status.NewErrorStatus(err)

// liveState is nil, fall back to create planObj directly
if liveState == nil {
if _, err = resource.Create(ctx, planObj, metav1.CreateOptions{}); err != nil {
return nil, status.NewErrorStatus(err)
}
} else {
// original equals to last-applied from annotation, kusion store it in kusion_state.json
original := ""
if priorState != nil {
original = json.MustMarshal2String(priorState.Attributes)
}
// modified equals input content
modified := json.MustMarshal2String(planState.Attributes)
// current equals live manifest
current := json.MustMarshal2String(liveState.Attributes)
// 3-way json merge patch
patch, err := jsonmergepatch.CreateThreeWayJSONMergePatch([]byte(original), []byte(modified), []byte(current))
if err != nil {
return nil, status.NewErrorStatus(err)
}
// apply patch
if _, err = resource.Patch(ctx, planObj.GetName(), types.MergePatchType, patch, metav1.PatchOptions{FieldManager: "kusion"}); err != nil {
return nil, status.NewErrorStatus(err)
}
}

return &models.Resource{
ID: planState.ResourceKey(),
Attributes: patchedObj.Object,
Attributes: planObj.Object,
DependsOn: planState.DependsOn,
}, nil
}
Expand All @@ -105,6 +113,10 @@ func (k *KubernetesRuntime) Read(ctx context.Context, resourceState *models.Reso
// Read resource
v, err := resource.Get(ctx, obj.GetName(), metav1.GetOptions{})
if err != nil {
if k8serrors.IsNotFound(err) {
log.Infof("%s not found, ignore", resourceState.ResourceKey())
return nil, nil
}
return nil, status.NewErrorStatus(err)
}

Expand Down
Loading

0 comments on commit d4d7245

Please sign in to comment.