Skip to content

Commit

Permalink
Use app label as app-change label for new apps (#710)
Browse files Browse the repository at this point in the history 
- Introduce a new label kapp.k14s.io/app-change-app-label for app changes which uses the app label as value
- Use the old label to list and delete app-changes created for exiting apps, but also add this new label to them.
- Use the new label to list and delete app-changes created for new apps, but add the old label if length of app name is less than the maximum allowed length of a label key

Signed-off-by: Praveen Rewar <[email protected]>
  • Loading branch information
@praveenrewar
praveenrewar authored Mar 6, 2023
1 parent 27c9ad7 commit 2b2475b
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 26 deletions.
26 changes: 18 additions & 8 deletions pkg/kapp/app/recorded_app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ const (
KappIsConfigmapMigratedAnnotationKey = "kapp.k14s.io/is-configmap-migrated"
KappIsConfigmapMigratedAnnotationValue = ""
AppSuffix = ".apps.k14s.io"
KappAppChangesUseAppLabelAnnotationKey = "kapp.k14s.io/app-changes-use-app-label"
)

type RecordedApp struct {
name string
nsName string
isMigrated bool
creationTimestamp time.Time
name string
nsName string
isMigrated bool
creationTimestamp time.Time
appChangesUseAppLabel bool

coreClient kubernetes.Interface
identifiedResources ctlres.IdentifiedResources
Expand All @@ -46,7 +48,7 @@ func NewRecordedApp(name, nsName string, creationTimestamp time.Time, coreClient
identifiedResources ctlres.IdentifiedResources, appInDiffNsHintMsgFunc func(string) string, logger logger.Logger) *RecordedApp {

// Always trim suffix, even if user added it manually (to avoid double migration)
return &RecordedApp{strings.TrimSuffix(name, AppSuffix), nsName, false, creationTimestamp, coreClient, identifiedResources, appInDiffNsHintMsgFunc,
return &RecordedApp{strings.TrimSuffix(name, AppSuffix), nsName, false, creationTimestamp, false, coreClient, identifiedResources, appInDiffNsHintMsgFunc,
nil, logger.NewPrefixed("RecordedApp")}
}

Expand Down Expand Up @@ -196,6 +198,10 @@ func (a *RecordedApp) create(labels map[string]string, isDiffRun bool) error {
Labels: map[string]string{
KappIsAppLabelKey: kappIsAppLabelValue,
},
// Use app label as app change label for all new apps
Annotations: map[string]string{
KappAppChangesUseAppLabelAnnotationKey: "",
},
},
Data: Meta{
LabelKey: kappAppLabelKey,
Expand Down Expand Up @@ -336,7 +342,7 @@ func (a *RecordedApp) Delete() error {
return err
}

err = NewRecordedAppChanges(a.nsName, a.name, meta.LabelValue, a.coreClient).DeleteAll()
err = NewRecordedAppChanges(a.nsName, a.name, meta.LabelValue, a.appChangesUseAppLabel, a.coreClient).DeleteAll()
if err != nil {
return fmt.Errorf("Deleting app changes: %w", err)
}
Expand Down Expand Up @@ -429,6 +435,9 @@ func (a *RecordedApp) isMigrationEnabled() bool {
func (a *RecordedApp) Meta() (Meta, error) { return a.meta() }

func (a *RecordedApp) setMeta(app corev1.ConfigMap) (Meta, error) {
// TODO: Should this be moved out of this function?
_, a.appChangesUseAppLabel = app.Annotations[KappAppChangesUseAppLabelAnnotationKey]

meta, err := NewAppMetaFromData(app.Data)
if err != nil {
errMsg := "App '%s' (namespace: %s) backed by ConfigMap '%s' did not contain parseable app metadata: %w"
Expand Down Expand Up @@ -479,7 +488,8 @@ func (a *RecordedApp) Changes() ([]Change, error) {
if err != nil {
return nil, err
}
return NewRecordedAppChanges(a.nsName, a.name, meta.LabelValue, a.coreClient).List()

return NewRecordedAppChanges(a.nsName, a.name, meta.LabelValue, a.appChangesUseAppLabel, a.coreClient).List()
}

func (a *RecordedApp) LastChange() (Change, error) {
Expand Down Expand Up @@ -508,7 +518,7 @@ func (a *RecordedApp) BeginChange(meta ChangeMeta) (Change, error) {
return nil, err
}

change, err := NewRecordedAppChanges(a.nsName, a.name, appMeta.LabelValue, a.coreClient).Begin(meta)
change, err := NewRecordedAppChanges(a.nsName, a.name, appMeta.LabelValue, a.appChangesUseAppLabel, a.coreClient).Begin(meta)
if err != nil {
return nil, err
}
Expand Down
55 changes: 37 additions & 18 deletions pkg/kapp/app/recorded_app_changes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,24 @@ import (
)

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

type RecordedAppChanges struct {
nsName string
appName string
nsName string
appName string
changeLabelValue string

appLabelValue string
appChangeUsesAppLabel bool

coreClient kubernetes.Interface
}

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

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

if !a.appChangeUsesAppLabel {
listOpts = metav1.ListOptions{
LabelSelector: labels.Set(map[string]string{
isChangeLabelKey: isChangeLabelValue,
legacyChangeLabelKey: a.appName,
}).String(),
}
}

changes, err := a.coreClient.CoreV1().ConfigMaps(a.nsName).List(context.TODO(), listOpts)
if err != nil {
return nil, err
Expand Down Expand Up @@ -73,10 +84,19 @@ func (a RecordedAppChanges) DeleteAll() error {
listOpts := metav1.ListOptions{
LabelSelector: labels.Set(map[string]string{
isChangeLabelKey: isChangeLabelValue,
changeLabelKey: a.changeLabelValue(),
changeLabelKey: a.changeLabelValue,
}).String(),
}

if !a.appChangeUsesAppLabel {
listOpts = metav1.ListOptions{
LabelSelector: labels.Set(map[string]string{
isChangeLabelKey: isChangeLabelValue,
legacyChangeLabelKey: a.appName,
}).String(),
}
}

changes, err := a.coreClient.CoreV1().ConfigMaps(a.nsName).List(context.TODO(), listOpts)
if err != nil {
return err
Expand Down Expand Up @@ -105,12 +125,18 @@ func (a RecordedAppChanges) Begin(meta ChangeMeta) (*ChangeImpl, error) {
Namespace: a.nsName,
Labels: map[string]string{
isChangeLabelKey: isChangeLabelValue,
changeLabelKey: a.changeLabelValue(),
changeLabelKey: a.changeLabelValue,
},
},
Data: newMeta.AsData(),
}

// Keep app changes backward compatible if possible, by adding legacy
// change label key when app name's length is less than maximum allowed length of a label
if !a.appChangeUsesAppLabel || len(a.appName) <= validation.LabelValueMaxLength {
configMap.ObjectMeta.Labels[legacyChangeLabelKey] = a.appName
}

createdChange, err := a.coreClient.CoreV1().ConfigMaps(a.nsName).Create(context.TODO(), configMap, metav1.CreateOptions{})
if err != nil {
return nil, fmt.Errorf("Creating app change: %w", err)
Expand All @@ -125,10 +151,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
}

0 comments on commit 2b2475b

Please sign in to comment.