Add condition helpers to Multi-Network API.

[mskrocki]

No description provided.

[mskrocki]

/cc @mrthinger

[k8s-ci-robot]

@mskrocki: GitHub didn't allow me to request PR reviews from the following users: mrthinger.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @mrthinger

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[mrthinger]

Could this be a method on the structs instead?

I don't think branching on type is a great way of implementing this, but this obviously works fine.

[aojea]

Could this be a method on the structs instead?

I just was inclined to avoid that #526 (comment), adding methods to the struct will add a dependency on the object that is versioned, and the function is agnostic of that, per example, you can use this function with existing version, but in order to use the method you have to update your dependency first

[aojea]

LGTM

I do not know if you prefer to use generics, cast interfaces or one function per object, but I have doubts on the method per object approach per #526 (comment), but I may be wrong too

[k8s-ci-robot]

@mskrocki: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cloud-provider-gcp-scenario-kops-simple	d4a33f9	link	false	/test pull-cloud-provider-gcp-scenario-kops-simple

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[mrthinger]

LGTM. The conditions function will be scoped to the network package anyway, so discoverability and maintainability shouldn't be an issue.

[MrHohn]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MrHohn, mrthinger, mskrocki

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• crd/OWNERS [MrHohn,mskrocki]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[bowei]

suggestion

create a struct that wraps metav1.Condition that implements the specific CRUD for conditions that we care about. Might be cleaner for users than one-off functions.

func NewConditionCRUD(c*metav1.Condition) ...
type ConditionCRUD struct { ... }


crud := NewConditionCRUD(obj.Status.Conditions)
if crud.Ready() { ...
}

crud.SetReady(false, "it's not ready")

[mskrocki]

As I were looking into this I found https://github.com/kubernetes/apimachinery/blob/master/pkg/api/meta/conditions.go which has all the required functions, will close this PR.

[mskrocki]

/hold

[mskrocki]

Using https://github.com/kubernetes/apimachinery/blob/master/pkg/api/meta/conditions.go instead.