Skip to content
This repository has been archived by the owner on Jun 8, 2022. It is now read-only.

Commit

Permalink
Merge pull request #296 from zzxwill/issue-revision
Browse files Browse the repository at this point in the history
Stop workload recreation if just RevisionEnabled trait attaching the first time
  • Loading branch information
wonderflow committed Nov 23, 2020
2 parents 40491a4 + a85cf24 commit 7c702eb
Show file tree
Hide file tree
Showing 5 changed files with 217 additions and 15 deletions.
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ require (
github.com/google/go-cmp v0.4.0
github.com/gophercloud/gophercloud v0.6.0 // indirect
github.com/json-iterator/go v1.1.10
github.com/kr/pretty v0.2.0 // indirect
github.com/onsi/ginkgo v1.12.1
github.com/onsi/gomega v1.10.1
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.1.0 // indirect
github.com/stretchr/testify v1.4.0
go.uber.org/zap v1.10.0
golang.org/x/tools v0.0.0-20200630223951-c138986dd9b9 // indirect
google.golang.org/appengine v1.6.5 // indirect
gopkg.in/natefinch/lumberjack.v2 v2.0.0
k8s.io/api v0.18.6
k8s.io/apiextensions-apiserver v0.18.6
Expand Down
10 changes: 10 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ github.com/PuerkitoBio/urlesc v0.0.0-20160726150825-5bd2802263f2/go.mod h1:uGdko
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 h1:d+Bc7a5rLufV/sSk/8dngufqelfh6jnri85riMAaF/M=
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE=
github.com/agnivade/levenshtein v1.0.1/go.mod h1:CURSv5d9Uaml+FovSIICkLbAUZ9S4RqaHDIsdSBg7lM=
github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc h1:cAKDfWh5VpdgMhJosfJnn5/FoN2SRZ4p7fJNX58YPaU=
github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf h1:qet1QNfXsQxTZqLG4oE62mJzwPIB8+Tee4RNCL9ulrY=
github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8=
github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
Expand Down Expand Up @@ -166,6 +168,7 @@ github.com/go-openapi/swag v0.19.5/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh
github.com/go-openapi/validate v0.18.0/go.mod h1:Uh4HdOzKt19xGIGm1qHf/ofbX1YQ4Y+MYsct2VUrAJ4=
github.com/go-openapi/validate v0.19.2/go.mod h1:1tRCw7m3jtI8eNWEEliiAqUIcBztB2KDnRCRMUi7GTA=
github.com/go-openapi/validate v0.19.5/go.mod h1:8DJv2CVJQ6kGNpFW6eV9N3JviE1C85nY1c2z52x1Gk4=
github.com/go-stack/stack v1.8.0 h1:5SgMzNM5HxrEjV0ww2lTmX6E2Izsfxas4+YHWRs3Lsk=
github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY=
github.com/gobuffalo/flect v0.1.5 h1:xpKq9ap8MbYfhuPCF0dBH854Gp9CxZjr/IocxELFflo=
github.com/gobuffalo/flect v0.1.5/go.mod h1:W3K3X9ksuZfir8f/LrfVtWmCDQFfayuylOJ7sz/Fj80=
Expand Down Expand Up @@ -270,10 +273,13 @@ github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7V
github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q=
github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00=
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
github.com/konsorten/go-windows-terminal-sequences v1.0.1 h1:mweAR1A6xJ3oS2pRaGiHgQ4OO8tzTaLawm8vnODuwDk=
github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc=
github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
github.com/kr/pretty v0.2.0 h1:s5hAObm+yFO5uHYt5dYjxi2rXrsnmRpJx4OYvIWUaQs=
github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
github.com/kr/pty v1.1.5/go.mod h1:9r2w37qlBe7rQ6e1fg1S/9xpWHSnaqNdHD3WcMdbPDA=
github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
Expand Down Expand Up @@ -370,6 +376,7 @@ github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6So
github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g=
github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo=
github.com/sirupsen/logrus v1.4.2 h1:SPIRibHv4MatM3XXNO2BJeFLZwZ2LvZgfQ5+UNI2im4=
github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=
github.com/soheilhy/cmux v0.1.3/go.mod h1:IM3LyeVVIOuxMH7sFAkER9+bJ4dT7Ms6E4xg4kGIyLM=
github.com/soheilhy/cmux v0.1.4/go.mod h1:IM3LyeVVIOuxMH7sFAkER9+bJ4dT7Ms6E4xg4kGIyLM=
Expand Down Expand Up @@ -567,6 +574,8 @@ google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7
google.golang.org/appengine v1.5.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
google.golang.org/appengine v1.6.1 h1:QzqyMA1tlu6CgqCDUtU9V+ZKhLFT2dkJuANu5QaxI3I=
google.golang.org/appengine v1.6.1/go.mod h1:i06prIuMbXzDqacNJfV5OdTW448YApPu5ww/cMBSeb0=
google.golang.org/appengine v1.6.5 h1:tycE03LOZYQNhDpS27tcQdAzLCVMaj7QT2SXxebnpCM=
google.golang.org/appengine v1.6.5/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc=
google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc=
google.golang.org/genproto v0.0.0-20190307195333-5fe7a883aa19/go.mod h1:VzzqZJRnGkLBvHegQrXjBqPurQTc5/KpmUdxsrq26oE=
google.golang.org/genproto v0.0.0-20190418145605-e7d98fc518a7/go.mod h1:VzzqZJRnGkLBvHegQrXjBqPurQTc5/KpmUdxsrq26oE=
Expand All @@ -589,6 +598,7 @@ google.golang.org/protobuf v1.20.1-0.20200309200217-e05f789c0967/go.mod h1:A+miE
google.golang.org/protobuf v1.21.0/go.mod h1:47Nbq4nVaFHyn7ilMalzfO3qCViNmqZ2kzikPIcrTAo=
google.golang.org/protobuf v1.23.0 h1:4MY060fB1DLGMB/7MBTLnwQUY6+F09GEiz6SsrNqyzM=
google.golang.org/protobuf v1.23.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU=
gopkg.in/alecthomas/kingpin.v2 v2.2.6 h1:jMFz6MfLP0/4fUyZle81rXUoxOBFi19VUFKVDOQfozc=
gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down
51 changes: 45 additions & 6 deletions pkg/controller/v1alpha2/applicationconfiguration/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ var (
ErrDataOutputNotExist = errors.New("DataOutput does not exist")
)

const instanceNamePath = "metadata.name"
const (
instanceNamePath = "metadata.name"
)

// A ComponentRenderer renders an ApplicationConfiguration's Components into
// workloads and traits.
Expand Down Expand Up @@ -138,7 +140,6 @@ func (r *components) renderComponent(ctx context.Context, acc v1alpha2.Applicati
if err != nil {
return nil, errors.Wrapf(err, errFmtRenderWorkload, acc.ComponentName)
}

compInfoLabels := map[string]string{
oam.LabelAppName: ac.Name,
oam.LabelAppComponent: acc.ComponentName,
Expand Down Expand Up @@ -176,7 +177,13 @@ func (r *components) renderComponent(ctx context.Context, acc v1alpha2.Applicati
traits = append(traits, &Trait{Object: *t, Definition: *traitDef})
traitDefs = append(traitDefs, *traitDef)
}
if err := SetWorkloadInstanceName(traitDefs, w, c); err != nil {

existingWorkload, err := r.getExistingWorkload(ctx, ac, c, w)
if err != nil {
return nil, err
}

if err := SetWorkloadInstanceName(traitDefs, w, c, existingWorkload); err != nil {
return nil, err
}
// create the ref after the workload name is set
Expand Down Expand Up @@ -257,7 +264,8 @@ func setTraitProperties(t *unstructured.Unstructured, traitName, namespace strin
}

// SetWorkloadInstanceName will set metadata.name for workload CR according to createRevision flag in traitDefinition
func SetWorkloadInstanceName(traitDefs []v1alpha2.TraitDefinition, w *unstructured.Unstructured, c *v1alpha2.Component) error {
func SetWorkloadInstanceName(traitDefs []v1alpha2.TraitDefinition, w *unstructured.Unstructured, c *v1alpha2.Component,
existingWorkload *unstructured.Unstructured) error {
// Don't override the specified name
if w.GetName() != "" {
return nil
Expand All @@ -267,10 +275,19 @@ func SetWorkloadInstanceName(traitDefs []v1alpha2.TraitDefinition, w *unstructur
if c.Status.LatestRevision == nil {
return fmt.Errorf(errFmtCompRevision, c.Name)
}
// if revisionEnabled, use revisionName as the workload name
if err := pv.SetString(instanceNamePath, c.Status.LatestRevision.Name); err != nil {

componentLastRevision := c.Status.LatestRevision.Name
// if workload exists, check the revision label, we will not change the name if workload exists and no revision changed
if existingWorkload != nil && existingWorkload.GetLabels()[oam.LabelAppComponentRevision] == componentLastRevision {
return nil
}

// if revisionEnabled and the running workload's revision isn't equal to the component's latest reversion,
// use revisionName as the workload name
if err := pv.SetString(instanceNamePath, componentLastRevision); err != nil {
return errors.Wrapf(err, errSetValueForField, instanceNamePath, c.Status.LatestRevision)
}

return nil
}
// use component name as workload name, which means we will always use one workload for different revisions
Expand Down Expand Up @@ -670,3 +687,25 @@ func getTraitName(ac *v1alpha2.ApplicationConfiguration, componentName string,

return traitName
}

// getExistingWorkload tries to retrieve the currently running workload
func (r *components) getExistingWorkload(ctx context.Context, ac *v1alpha2.ApplicationConfiguration, c *v1alpha2.Component, w *unstructured.Unstructured) (*unstructured.Unstructured, error) {
var workloadName string
existingWorkload := &unstructured.Unstructured{}
for _, component := range ac.Status.Workloads {
if component.ComponentName != c.GetName() {
continue
}
workloadName = component.Reference.Name
}
if workloadName != "" {
objectKey := client.ObjectKey{Namespace: ac.GetNamespace(), Name: workloadName}
existingWorkload.SetAPIVersion(w.GetAPIVersion())
existingWorkload.SetKind(w.GetKind())
err := r.client.Get(ctx, objectKey, existingWorkload)
if err != nil {
return nil, client.IgnoreNotFound(err)
}
}
return existingWorkload, nil
}
48 changes: 39 additions & 9 deletions pkg/controller/v1alpha2/applicationconfiguration/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -922,12 +922,13 @@ func TestGetDefinitionName(t *testing.T) {

func TestSetWorkloadInstanceName(t *testing.T) {
tests := map[string]struct {
traitDefs []v1alpha2.TraitDefinition
u *unstructured.Unstructured
c *v1alpha2.Component
exp *unstructured.Unstructured
expErr error
reason string
traitDefs []v1alpha2.TraitDefinition
u *unstructured.Unstructured
c *v1alpha2.Component
exp *unstructured.Unstructured
expErr error
currentWorkload *unstructured.Unstructured
reason string
}{
"with a name, no change": {
u: &unstructured.Unstructured{Object: map[string]interface{}{
Expand All @@ -943,14 +944,43 @@ func TestSetWorkloadInstanceName(t *testing.T) {
}},
reason: "workloadName should not change if already set",
},
"revisionEnabled true, set revisionName": {
"revisionEnabled true, and revision differs to the existing one, use new revisionName": {
traitDefs: []v1alpha2.TraitDefinition{
{
Spec: v1alpha2.TraitDefinitionSpec{RevisionEnabled: true},
},
},
u: &unstructured.Unstructured{Object: map[string]interface{}{}},
c: &v1alpha2.Component{ObjectMeta: metav1.ObjectMeta{Name: "comp"}, Status: v1alpha2.ComponentStatus{LatestRevision: &v1alpha2.Revision{Name: "rev-2"}}},
currentWorkload: &unstructured.Unstructured{Object: map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]string{
"app.oam.dev/revision": "rev-v1",
},
},
}},
exp: &unstructured.Unstructured{Object: map[string]interface{}{
"metadata": map[string]interface{}{
"name": "rev-2",
},
}},
reason: "workloadName should align with new revisionName",
},
"revisionEnabled true, and revision is same with the existing one, keep the old name": {
traitDefs: []v1alpha2.TraitDefinition{
{
Spec: v1alpha2.TraitDefinitionSpec{RevisionEnabled: true},
},
},
u: &unstructured.Unstructured{Object: map[string]interface{}{}},
c: &v1alpha2.Component{ObjectMeta: metav1.ObjectMeta{Name: "comp"}, Status: v1alpha2.ComponentStatus{LatestRevision: &v1alpha2.Revision{Name: "rev-1"}}},
currentWorkload: &unstructured.Unstructured{Object: map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]string{
"app.oam.dev/revision": "rev-v1",
},
},
}},
exp: &unstructured.Unstructured{Object: map[string]interface{}{
"metadata": map[string]interface{}{
"name": "rev-1",
Expand Down Expand Up @@ -996,9 +1026,9 @@ func TestSetWorkloadInstanceName(t *testing.T) {
for name, ti := range tests {
t.Run(name, func(t *testing.T) {
if ti.expErr != nil {
assert.Equal(t, ti.expErr.Error(), SetWorkloadInstanceName(ti.traitDefs, ti.u, ti.c).Error())
assert.Equal(t, ti.expErr.Error(), SetWorkloadInstanceName(ti.traitDefs, ti.u, ti.c, ti.currentWorkload).Error())
} else {
err := SetWorkloadInstanceName(ti.traitDefs, ti.u, ti.c)
err := SetWorkloadInstanceName(ti.traitDefs, ti.u, ti.c, ti.currentWorkload)
assert.NoError(t, err)
assert.Equal(t, ti.exp, ti.u, ti.reason)
}
Expand Down
121 changes: 121 additions & 0 deletions test/e2e-test/component_version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util"

appsv1 "k8s.io/api/apps/v1"
v1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -490,3 +491,123 @@ func readYaml(path string, object runtime.Object) error {
}
return yaml.Unmarshal(data, object)
}

var _ = Describe("Component revision", func() {
ctx := context.Background()
apiVersion := "core.oam.dev/v1alpha2"
namespace := "default"
componentName := "revision-component"
appConfigName := "revision-app"
workload := v1.Deployment{
TypeMeta: metav1.TypeMeta{APIVersion: "apps/v1", Kind: "Deployment"},
ObjectMeta: metav1.ObjectMeta{Namespace: namespace},
Spec: v1.DeploymentSpec{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"app": "nginx",
},
},
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{Name: "nginx",
Image: "nginx:1.9.4"},
},
},
ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"app": "nginx"}},
},
},
}
component := v1alpha2.Component{
TypeMeta: metav1.TypeMeta{
APIVersion: apiVersion,
Kind: "Component",
},
ObjectMeta: metav1.ObjectMeta{Name: componentName, Namespace: namespace},
Spec: v1alpha2.ComponentSpec{
Workload: runtime.RawExtension{Object: workload.DeepCopyObject()},
},
}

TraitDefinition := v1alpha2.TraitDefinition{
TypeMeta: metav1.TypeMeta{
APIVersion: apiVersion,
Kind: "TraitDefinition",
},
ObjectMeta: metav1.ObjectMeta{
Name: "manualscalertraits2.core.oam.dev",
Namespace: namespace,
},
Spec: v1alpha2.TraitDefinitionSpec{
RevisionEnabled: true,
Reference: v1alpha2.DefinitionReference{
Name: "manualscalertraits.core.oam.dev",
},
WorkloadRefPath: "spec.workloadRef",
},
}

appConfig := v1alpha2.ApplicationConfiguration{
TypeMeta: metav1.TypeMeta{
APIVersion: apiVersion,
Kind: "ApplicationConfiguration",
},
ObjectMeta: metav1.ObjectMeta{Name: appConfigName, Namespace: namespace},
Spec: v1alpha2.ApplicationConfigurationSpec{
Components: []v1alpha2.ApplicationConfigurationComponent{{
ComponentName: componentName},
},
},
}

workloadObjKey := client.ObjectKey{Name: componentName, Namespace: namespace}
appConfigObjKey := client.ObjectKey{Name: appConfigName, Namespace: namespace}

trait := v1alpha2.ManualScalerTrait{
TypeMeta: metav1.TypeMeta{
APIVersion: apiVersion,
Kind: "ManualScalerTrait",
},
ObjectMeta: metav1.ObjectMeta{Name: appConfigName, Namespace: namespace},
Spec: v1alpha2.ManualScalerTraitSpec{
ReplicaCount: 2,
},
}

Context("Attach a revision-enable trait the first time, workload should not be recreated", func() {
It("should create Component and ApplicationConfiguration", func() {
By("submit ApplicationConfiguration")
Expect(k8sClient.Create(ctx, &component)).Should(Succeed())
Expect(k8sClient.Create(ctx, &appConfig)).Should(Succeed())

By("check workload")
var deploy v1.Deployment
Eventually(
func() error {
return k8sClient.Get(ctx, workloadObjKey, &deploy)
},
time.Second*15, time.Millisecond*500).Should(BeNil())

By("apply new ApplicationConfiguration with a revision enabled trait")
Expect(k8sClient.Create(ctx, &TraitDefinition)).Should(Succeed())
Expect(k8sClient.Get(ctx, appConfigObjKey, &appConfig)).Should(Succeed())
appConfig.Spec.Components[0].Traits = []v1alpha2.ComponentTrait{{Trait: runtime.RawExtension{Object: trait.DeepCopyObject()}}}
Expect(k8sClient.Update(ctx, &appConfig)).Should(Succeed())

By("check current workload exists")
time.Sleep(3 * time.Second)
var currentDeploy v1.Deployment
Expect(k8sClient.Get(ctx, workloadObjKey, &currentDeploy)).Should(BeNil())

By("check version 1 workload doesn't exist")
var v1Deploy v1.Deployment
workloadObjKey := client.ObjectKey{Name: componentName + "-v1", Namespace: namespace}
Expect(k8sClient.Get(ctx, workloadObjKey, &v1Deploy)).Should(SatisfyAny(&util.NotFoundMatcher{}))
})
})

AfterEach(func() {
k8sClient.Delete(ctx, &appConfig)
k8sClient.Delete(ctx, &component)
})
})

0 comments on commit 7c702eb

Please sign in to comment.