Skip to content

Commit

Permalink
Revert "Use app label as app change label for long app names (#685) (#…
Browse files Browse the repository at this point in the history
…691)" (#699)

This reverts commit 1d1eb96.

Signed-off-by: Praveen Rewar <[email protected]>
  • Loading branch information
praveenrewar authored Feb 7, 2023
1 parent 9e1ece9 commit 975b459
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 80 deletions.
20 changes: 3 additions & 17 deletions pkg/kapp/app/recorded_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,12 +359,7 @@ func (a *RecordedApp) Delete() error {
return err
}

meta, err := a.meta()
if err != nil {
return err
}

err = NewRecordedAppChanges(a.nsName, a.name, meta.LabelValue, a.coreClient).DeleteAll()
err = NewRecordedAppChanges(a.nsName, a.name, a.coreClient).DeleteAll()
if err != nil {
return fmt.Errorf("Deleting app changes: %w", err)
}
Expand Down Expand Up @@ -503,11 +498,7 @@ func (a *RecordedApp) meta() (Meta, error) {
}

func (a *RecordedApp) Changes() ([]Change, error) {
meta, err := a.meta()
if err != nil {
return nil, err
}
return NewRecordedAppChanges(a.nsName, a.name, meta.LabelValue, a.coreClient).List()
return NewRecordedAppChanges(a.nsName, a.name, a.coreClient).List()
}

func (a *RecordedApp) LastChange() (Change, error) {
Expand All @@ -531,12 +522,7 @@ func (a *RecordedApp) LastChange() (Change, error) {
}

func (a *RecordedApp) BeginChange(meta ChangeMeta) (Change, error) {
appMeta, err := a.meta()
if err != nil {
return nil, err
}

change, err := NewRecordedAppChanges(a.nsName, a.name, appMeta.LabelValue, a.coreClient).Begin(meta)
change, err := NewRecordedAppChanges(a.nsName, a.name, a.coreClient).Begin(meta)
if err != nil {
return nil, err
}
Expand Down
22 changes: 6 additions & 16 deletions pkg/kapp/app/recorded_app_changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,24 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/client-go/kubernetes"
)

const (
isChangeLabelKey = "kapp.k14s.io/is-app-change"
isChangeLabelValue = ""
changeLabelKey = "kapp.k14s.io/app-change-app" // holds app name or label
changeLabelKey = "kapp.k14s.io/app-change-app" // holds app name
)

type RecordedAppChanges struct {
nsName string
appName string

appLabelValue string

coreClient kubernetes.Interface
}

func NewRecordedAppChanges(nsName, appName, appLabelValue string, coreClient kubernetes.Interface) RecordedAppChanges {
return RecordedAppChanges{nsName, appName, appLabelValue, coreClient}
func NewRecordedAppChanges(nsName, appName string, coreClient kubernetes.Interface) RecordedAppChanges {
return RecordedAppChanges{nsName, appName, coreClient}
}

func (a RecordedAppChanges) List() ([]Change, error) {
Expand All @@ -41,7 +38,7 @@ func (a RecordedAppChanges) List() ([]Change, error) {
listOpts := metav1.ListOptions{
LabelSelector: labels.Set(map[string]string{
isChangeLabelKey: isChangeLabelValue,
changeLabelKey: a.changeLabelValue(),
changeLabelKey: a.appName,
}).String(),
}

Expand Down Expand Up @@ -73,7 +70,7 @@ func (a RecordedAppChanges) DeleteAll() error {
listOpts := metav1.ListOptions{
LabelSelector: labels.Set(map[string]string{
isChangeLabelKey: isChangeLabelValue,
changeLabelKey: a.changeLabelValue(),
changeLabelKey: a.appName,
}).String(),
}

Expand Down Expand Up @@ -105,7 +102,7 @@ func (a RecordedAppChanges) Begin(meta ChangeMeta) (*ChangeImpl, error) {
Namespace: a.nsName,
Labels: map[string]string{
isChangeLabelKey: isChangeLabelValue,
changeLabelKey: a.changeLabelValue(),
changeLabelKey: a.appName,
},
},
Data: newMeta.AsData(),
Expand All @@ -125,10 +122,3 @@ func (a RecordedAppChanges) Begin(meta ChangeMeta) (*ChangeImpl, error) {

return change, nil
}

func (a RecordedAppChanges) changeLabelValue() string {
if len(a.appName) > validation.LabelValueMaxLength {
return a.appLabelValue
}
return a.appName
}
47 changes: 0 additions & 47 deletions test/e2e/app_change_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,53 +93,6 @@ spec:
})
}

func TestAppChangeWithLongAppName(t *testing.T) {
env := BuildEnv(t)
logger := Logger{}
kapp := Kapp{t, env.Namespace, env.KappBinaryPath, logger}

yaml := `
---
apiVersion: v1
kind: Service
metadata:
name: redis-primary
spec:
ports:
- port: 6380
targetPort: 6380
selector:
app: redis
tier: %s
`

name := "test-app-change-with-a-very-very-very-very-very-very-very-long-app-name"
cleanUp := func() {
kapp.Run([]string{"delete", "-a", name})
}

cleanUp()
defer cleanUp()

logger.Section("deploy app", func() {
kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name}, RunOpts{IntoNs: true, StdinReader: strings.NewReader(fmt.Sprintf(yaml, "backend"))})
})

logger.Section("deploy with changes", func() {
kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name}, RunOpts{IntoNs: true, StdinReader: strings.NewReader(fmt.Sprintf(yaml, "frontend"))})
})

logger.Section("app change list", func() {
out, _ := kapp.RunWithOpts([]string{"app-change", "ls", "-a", name, "--json"}, RunOpts{})

resp := uitest.JSONUIFromBytes(t, []byte(out))

require.Equal(t, 2, len(resp.Tables[0].Rows), "Expected to have 2 app-changes")
require.Equal(t, "update: Op: 0 create, 0 delete, 1 update, 0 noop, 0 exists / Wait to: 1 reconcile, 0 delete, 0 noop", resp.Tables[0].Rows[0]["description"], "Expected description to match")
require.Equal(t, "update: Op: 1 create, 0 delete, 0 update, 0 noop, 0 exists / Wait to: 1 reconcile, 0 delete, 0 noop", resp.Tables[0].Rows[1]["description"], "Expected description to match")
})
}

func TestAppKindChangeWithMetadataOutput(t *testing.T) {
env := BuildEnv(t)
logger := Logger{}
Expand Down

0 comments on commit 975b459

Please sign in to comment.