kpt icon indicating copy to clipboard operation
kpt copied to clipboard

Adding condition methods to the kpt file

Open henderiw opened this issue 1 year ago • 2 comments

Describe your problem

Could it be possible to add a number of condition methods to the kptfile?

I did a small library to do this but it make more sense if this is part of the standard library.

here is the code.

// SetConditions sets the conditions in the kptfile. It either updates the entry if it exists // or appends the entry if it does not exist. func (r *kptFile) SetConditions(c ...kptv1.Condition) { // validate is the status is set, if not initialize the condition slice if r.GetKptFile().Status == nil { r.GetKptFile().Status = &kptv1.Status{ Conditions: []kptv1.Condition{}, } }

// for each nex condition check if the kind
for _, new := range c {
	exists := false
	for i, existing := range r.GetKptFile().Status.Conditions {
		if existing.Type != new.Type {
			continue
		}
		r.GetKptFile().Status.Conditions[i] = new
		exists = true
	}
	if !exists {
		r.GetKptFile().Status.Conditions = append(r.GetKptFile().Status.Conditions, new)
	}
}

}

// DeleteCondition deletes the condition equal to the conditionType if it exists func (r *kptFile) DeleteCondition(ct string) { if r.GetKptFile().Status == nil || len(r.GetKptFile().Status.Conditions) == 0 { return }

for idx, c := range r.GetKptFile().Status.Conditions {
	if c.Type == ct {
		r.GetKptFile().Status.Conditions = append(r.GetKptFile().Status.Conditions[:idx], r.GetKptFile().Status.Conditions[idx+1:]...)
	}
}

}

// GetCondition returns the condition for the given ConditionType if it exists, // otherwise returns nil func (r *kptFile) GetCondition(ct string) *kptv1.Condition { if r.GetKptFile().Status == nil || len(r.GetKptFile().Status.Conditions) == 0 { return nil }

for _, c := range r.GetKptFile().Status.Conditions {
	if c.Type == ct {
		return &c
	}
}
return nil

}

// GetConditions returns all the conditions in the kptfile. if not initialized it // returns an emoty slice func (r *kptFile) GetConditions() []kptv1.Condition { if r.GetKptFile().Status == nil || len(r.GetKptFile().Status.Conditions) == 0 { return []kptv1.Condition{} } return r.GetKptFile().Status.Conditions }

tests:

func TestGetCondition(t *testing.T) {

f := `apiVersion: kpt.dev/v1

kind: Kptfile metadata: name: pkg-upf annotations: config.kubernetes.io/local-config: "true" info: description: upf package example `

kf := NewMutator(f)
if _, err := kf.UnMarshal(); err != nil {
	t.Errorf("cannot unmarshal file: %s", err.Error())
}

cases := map[string]struct {
	cs   []kptv1.Condition
	t    string
	want *kptv1.Condition
}{
	"ConditionExists": {
		cs: []kptv1.Condition{
			{Type: "b", Status: kptv1.ConditionFalse, Reason: "b", Message: "b"},
			{Type: "c", Status: kptv1.ConditionFalse, Reason: "c", Message: "c"},
			{Type: "a", Status: kptv1.ConditionFalse, Reason: "a", Message: "a"},
		},
		t:    "a",
		want: &kptv1.Condition{Type: "a", Status: kptv1.ConditionFalse, Reason: "a", Message: "a"},
	},
	"ConditionDoesNotExist": {
		cs: []kptv1.Condition{
			{Type: "b", Status: kptv1.ConditionFalse, Reason: "b", Message: "b"},
			{Type: "c", Status: kptv1.ConditionFalse, Reason: "c", Message: "c"},
			{Type: "a", Status: kptv1.ConditionFalse, Reason: "a", Message: "a"},
		},
		t:    "x",
		want: nil,
	},
	"ConditionEmptyList": {
		cs:   nil,
		t:    "x",
		want: nil,
	},
}

for name, tc := range cases {
	t.Run(name, func(t *testing.T) {
		if tc.cs != nil {
			kf.SetConditions(tc.cs...)
		}
		got := kf.GetCondition(tc.t)
		if got == nil || tc.want == nil {
			if got != tc.want {
				t.Errorf("TestGetCondition: -want%s, +got:\n%s", tc.want, got)
			}
		} else {
			if diff := cmp.Diff(tc.want, got); diff != "" {
				t.Errorf("TestGetCondition: -want, +got:\n%s", diff)
			}
		}

	})
}

}

func TestSetConditions(t *testing.T) {

f := `apiVersion: kpt.dev/v1

kind: Kptfile metadata: name: pkg-upf annotations: config.kubernetes.io/local-config: "true" info: description: upf package example `

cases := map[string]struct {
	cs   []kptv1.Condition
	t    []kptv1.Condition
	want []kptv1.Condition
}{
	"SetConditionsEmpty": {
		cs: nil,
		t: []kptv1.Condition{
			{Type: "b", Status: kptv1.ConditionFalse, Reason: "b", Message: "b"},
			{Type: "c", Status: kptv1.ConditionFalse, Reason: "c", Message: "c"},
			{Type: "a", Status: kptv1.ConditionFalse, Reason: "a", Message: "a"},
		},
		want: []kptv1.Condition{
			{Type: "b", Status: kptv1.ConditionFalse, Reason: "b", Message: "b"},
			{Type: "c", Status: kptv1.ConditionFalse, Reason: "c", Message: "c"},
			{Type: "a", Status: kptv1.ConditionFalse, Reason: "a", Message: "a"},
		},
	},
	"SetConditionsNonEmpty": {
		cs: []kptv1.Condition{
			{Type: "x", Status: kptv1.ConditionFalse, Reason: "x", Message: "x"},
			{Type: "y", Status: kptv1.ConditionFalse, Reason: "y", Message: "y"},
		},
		t: []kptv1.Condition{
			{Type: "b", Status: kptv1.ConditionFalse, Reason: "b", Message: "b"},
			{Type: "c", Status: kptv1.ConditionFalse, Reason: "c", Message: "c"},
			{Type: "a", Status: kptv1.ConditionFalse, Reason: "a", Message: "a"},
		},
		want: []kptv1.Condition{
			{Type: "x", Status: kptv1.ConditionFalse, Reason: "x", Message: "x"},
			{Type: "y", Status: kptv1.ConditionFalse, Reason: "y", Message: "y"},
			{Type: "b", Status: kptv1.ConditionFalse, Reason: "b", Message: "b"},
			{Type: "c", Status: kptv1.ConditionFalse, Reason: "c", Message: "c"},
			{Type: "a", Status: kptv1.ConditionFalse, Reason: "a", Message: "a"},
		},
	},
	"SetConditionsOverlap": {
		cs: []kptv1.Condition{
			{Type: "x", Status: kptv1.ConditionFalse, Reason: "x", Message: "x"},
			{Type: "y", Status: kptv1.ConditionFalse, Reason: "y", Message: "y"},
		},
		t: []kptv1.Condition{
			{Type: "b", Status: kptv1.ConditionFalse, Reason: "b", Message: "b"},
			{Type: "y", Status: kptv1.ConditionFalse, Reason: "ynew", Message: "ynew"},
			{Type: "a", Status: kptv1.ConditionFalse, Reason: "a", Message: "a"},
		},
		want: []kptv1.Condition{
			{Type: "x", Status: kptv1.ConditionFalse, Reason: "x", Message: "x"},
			{Type: "y", Status: kptv1.ConditionFalse, Reason: "ynew", Message: "ynew"},
			{Type: "b", Status: kptv1.ConditionFalse, Reason: "b", Message: "b"},
			{Type: "a", Status: kptv1.ConditionFalse, Reason: "a", Message: "a"},
		},
	},
}

for name, tc := range cases {
	kf := NewMutator(f)
	if _, err := kf.UnMarshal(); err != nil {
		t.Errorf("cannot unmarshal file: %s", err.Error())
	}
	t.Run(name, func(t *testing.T) {
		if tc.cs != nil {
			kf.SetConditions(tc.cs...)
		}
		if tc.t != nil {
			kf.SetConditions(tc.t...)
		}
		gots := kf.GetConditions()
		if len(gots) != len(tc.want) {
			t.Errorf("TestSetConditions: got: %v, want: %v", gots, tc.want)
		} else {
			for idx, got := range gots {
				// no need to validate length as this was already done
				/*
					if idx > len(tc.want)-1 {
						t.Errorf("TestSetConditions: got: %v, want: %v", gots, tc.want)
					}
				*/
				if diff := cmp.Diff(tc.want[idx], got); diff != "" {
					t.Errorf("TestGetCondition: -want, +got:\n%s", diff)
				}
			}
		}
	})
}

}

henderiw avatar Mar 28 '23 14:03 henderiw

Yeah, I agree this would be useful. If we aligned the conditions with meta.condition as you suggested in https://github.com/GoogleContainerTools/kpt/issues/3657 we probably wouldn't need it, but I haven't had a chance to look into that yet.

mortent avatar Mar 29 '23 18:03 mortent

@henderiw, this is the code you want to upstream at some point, correct?

https://github.com/nephio-project/nephio/tree/main/krm-functions/lib/kptfile/v1

johnbelamaric avatar Apr 04 '23 20:04 johnbelamaric