search

nephio-project / nephio

Nephio is a Kubernetes-based automation platform for deploying and managing highly distributed, interconnected workloads such as 5G Network Functions, and the underlying infrastructure on which those workloads depend.
Apache License 2.0
93 stars 52 forks source link

Adding condition methods to the kpt file #611

Closed liamfallon closed 1 month ago

liamfallon commented 2 months ago

Original issue URL: https://github.com/kptdev/kpt/issues/3896 Original issue user: https://github.com/henderiw Original issue created at: 2023-03-28T14:58:17Z Original issue last updated at: 2023-04-04T20:46:21Z Original issue body: ### 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)
                }
            }
        }
    })
}

}

Original issue comments: Comment user: https://github.com/mortent Comment created at: 2023-03-29T18:05:15Z Comment last updated at: 2023-03-29T18:05:15Z Comment body: 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.

Comment user: https://github.com/johnbelamaric Comment created at: 2023-04-04T20:46:21Z Comment last updated at: 2023-04-04T20:46:21Z Comment body: @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

liamfallon commented 1 month ago

Triage Comments: Nice to have. This is mostly already there. The idea was to avoid adding libraries but in the meantime a different approach was taken.