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

Is it not better to lineup the condition with the meta.condition #701

Open liamfallon opened 3 months ago

liamfallon commented 3 months ago

Original issue URL: https://github.com/kptdev/kpt/issues/3657 Original issue user: https://github.com/henderiw Original issue created at: 2022-11-14T10:18:01Z Original issue last updated at: 2022-11-15T21:37:11Z Original issue body: ### Expected behavior

right now we have to translate from porch.condition to metav1 condition as there are many functions available through the metav1.condition we can reuse. We can also have the last transition time in it to keep track what changed when.

Actual behavior

right now we translate from one to another so we can avoid this

Information

func convertConditions(conditions []porchv1alpha1.Condition) *[]metav1.Condition { var result []metav1.Condition for _, c := range conditions { result = append(result, metav1.Condition{ Type: c.Type, Reason: c.Reason, Status: metav1.ConditionStatus(c.Status), Message: c.Message, }) } return &result }

func unconvertConditions(conditions []metav1.Condition) []porchv1alpha1.Condition { var prConditions []porchv1alpha1.Condition for _, c := range conditions { prConditions = append(prConditions, porchv1alpha1.Condition{ Type: c.Type, Reason: c.Reason, Status: porchv1alpha1.ConditionStatus(c.Status), Message: c.Message, }) }

return prConditions

}

@johnbelamaric @mortent might be good to get your perspective on this

Steps to reproduce the behavior

Original issue comments: Comment user: https://github.com/mortent Comment created at: 2022-11-14T22:29:21Z Comment last updated at: 2022-11-14T22:29:21Z Comment body: yeah, we can consider using the metav1.Condition type. My main concern about it is that it includes the LastTransitionType property as a required value. For a controller this is probably not an issue, but we mirror the conditions into the Kptfile, and filling out the timestamp manually is somewhat awkward.