Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bgpv1: Consolidate CRD API to follow K8s API Conventions #26040

Merged
merged 4 commits into from
Jun 15, 2023
Merged

bgpv1: Consolidate CRD API to follow K8s API Conventions #26040

merged 4 commits into from
Jun 15, 2023

Commits on Jun 15, 2023

  1. bgpv1: Represent duration fields as integer fields in CRD API 

    Modifies the type of recently added CRD fields of type
`meta/v1.Duration` to integer type and their name to follow
the K8s API Conventions rule:

"Duration fields must be represented as integer fields with
units being part of the field name (e.g. leaseDurationSeconds).
We don't use Duration in the API since that would require clients
to implement go-compatible parsing."

Also enables defaulting for these fields on apiserver by using
the kubebuilder:default tags (for static defaults).

This includes the KeepaliveTimeSeconds field, which was defaulted
at runtime to 1/3 of HoldTimeSeconds. As per K8s API Conventions,
static defaults are preferred. It also makes the value more
transparent to the user.

Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
    @rastislavs
    rastislavs committed Jun 15, 2023
    Configuration menu
    Copy the full SHA
    15fded0 View commit details
    Browse the repository at this point in the history

  2. bgpv1: Move constants with default values to the API package 

    This allows the default value to be versioned with the API.
It also provides consistency with the kubebuilder tags that
define the default values - both are kept in the same source file.

Also moves timers validation logic to API package to co-locate
the validation logic for API constraints that cannot be
expressed with kubebuilder markers with the API itself.

Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
    @rastislavs
    rastislavs committed Jun 15, 2023
    Configuration menu
    Copy the full SHA
    1ce594e View commit details
    Browse the repository at this point in the history

  3. bgpv1: Consolidate CRD field types to follow K8s API Conventions 

    Modifies the types of CRD fileds to follow  the K8s API
Conventions rules:
- "Use pointer types for all optional fields that do not have
built-in nil value"
- "Do not use unsigned integers, due to inconsistent support
across languages and libraries"
- "All public integer fields MUST use the Go (u)int32 or Go
 (u)int64 types, not (u)int"

Also changes the types of the existing ASN CRD fileds to int64
to satisfy the above rules.

Defaults ExportPodCIDR field of CiliumBGPVirtualRouter to false
explicitly for consistency with other optional fields.
As the default behavior remains the same, this is
a backward-compatible change.

Pointer types can be directly dereferenced in the reconcilers code.
To allow that we provide defaulting methods for our API types,
that can be used from the controllers / unit tests.

Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
    @rastislavs
    rastislavs committed Jun 15, 2023
    Configuration menu
    Copy the full SHA
    e00559c View commit details
    Browse the repository at this point in the history

  4. bgpv1: Add unit tests for BGP CRD types 

    Add unit tests to newly added SetDefaults() and Validate()
methods of the BGP CRD types. Do not perform detailed
defaulting / validation checks in unit tests of other packages,
to have that testing logic co-located within a single package.

Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
    @rastislavs
    rastislavs committed Jun 15, 2023
    Configuration menu
    Copy the full SHA
    ff34296 View commit details
    Browse the repository at this point in the history