Skip to content

Conversation

@abhat
Copy link
Contributor

@abhat abhat commented Nov 25, 2019

Add missing annotations and validation markers for network types.

@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Nov 25, 2019
@openshift-ci-robot
Copy link

@abhat: This pull request references Bugzilla bug 1769015, which is valid. The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Bug 1769015: Add kubebuilder annotations to the network types

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.

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 25, 2019
@abhat
Copy link
Contributor Author

abhat commented Nov 25, 2019

/assign @danwinship

@abhat abhat force-pushed the annotate_nw_types branch 3 times, most recently from 1b10f56 to 0d43a1c Compare November 25, 2019 20:57
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few elements missing:

  1. You need to copy the crd yamls from network-operator into network/v1 directory, similarly how it's done in operator/v1, for example.
  2. You need to add this rule to Makefile:
$(call add-crd-gen,network,./network/v1,./network/v1,./network/v1)

next to similar lines for operator.

The above 2 steps will ensure we will update the yamls inside this project, and you'll be able to consume it nicely in openshift/cluster-network-operator#398

Copy link
Contributor

@danwinship danwinship left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you diff the original hand-written CRD against the new generated one to make sure there are no other unexpected changes?

@soltysh
Copy link
Contributor

soltysh commented Dec 16, 2019

My earlier comments still hold.

@abhat
Copy link
Contributor Author

abhat commented Jan 7, 2020

There are a few elements missing:

1. You need to copy the crd yamls from network-operator into `network/v1` directory, similarly how it's done in `operator/v1`, for example.

2. You need to add this rule to Makefile:
$(call add-crd-gen,network,./network/v1,./network/v1,./network/v1)

next to similar lines for operator.

The above 2 steps will ensure we will update the yamls inside this project, and you'll be able to consume it nicely in openshift/cluster-network-operator#398

For 1, is there a naming scheme convention? Right now the file in the cluster-network-operator repo is called 001-crd.yaml.

I have made the change for calling add-crd-gen

@abhat abhat force-pushed the annotate_nw_types branch from f3e80d5 to 7ce7d4a Compare January 7, 2020 20:11
@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 7, 2020
@abhat abhat changed the base branch from release-4.3 to master January 7, 2020 20:11
@openshift-ci-robot
Copy link

@abhat: This pull request references Bugzilla bug 1769015, which is valid.

Details

In response to this:

Bug 1769015: Add kubebuilder annotations to the network types

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.

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 7, 2020
@abhat
Copy link
Contributor Author

abhat commented Jan 7, 2020

/test verify

@sttts
Copy link
Contributor

sttts commented Jan 10, 2020

@abhat abhat force-pushed the annotate_nw_types branch 2 times, most recently from e43bb9f to 73cd943 Compare January 10, 2020 17:58
@abhat
Copy link
Contributor Author

abhat commented Jan 10, 2020

@soltysh @sttts can one of you approve and lgtm this?

Address review comments
Copy the crd yaml into the network directory
@abhat abhat force-pushed the annotate_nw_types branch from 73cd943 to 2b89f31 Compare January 16, 2020 15:29
@knobunc
Copy link
Contributor

knobunc commented Jan 22, 2020

@soltysh @sttts is this good to go?

Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhat, soltysh

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 23, 2020
@openshift-merge-robot openshift-merge-robot merged commit f263157 into openshift:master Jan 23, 2020
danwinship added a commit to danwinship/cluster-kube-controller-manager-operator that referenced this pull request Mar 2, 2020
In particular, to get

openshift/api#585: config: disable IPv6DualStack feature flag

so we don't launch kube-controller-manager with that (still-broken) feature

Also includes:

openshift/api#557: create the IBMCLoudPlatform type for the ingress operator try2
openshift/api#570: Clarify image config doc
openshift/api#569: Enable overriding service account issuer for bound tokens
openshift/api#527: Add kubebuilder annotations to the network types
openshift/api#574: add deprecaction notice for build pipeline strategy
openshift/api#582: config/v1/types_proxy: Clarify trustedCA semantics
openshift/api#583: Clarify FROM behavior in builds
openshift/api#573: Add CRD generator documentation to Readme
openshift/api#576: Remove Description from CLI output to improve its display
openshift/api#589: Add missing enum validations
openshift/api#583: operator/ingress: add dnsrecord type
soltysh added a commit to soltysh/cluster-kube-controller-manager-operator that referenced this pull request Mar 4, 2020
In particular, to get

openshift/api#585: config: disable IPv6DualStack feature flag

so we don't launch kube-controller-manager with that (still-broken) feature

Also includes:

openshift/api#557: create the IBMCLoudPlatform type for the ingress operator try2
openshift/api#570: Clarify image config doc
openshift/api#569: Enable overriding service account issuer for bound tokens
openshift/api#527: Add kubebuilder annotations to the network types
openshift/api#574: add deprecaction notice for build pipeline strategy
openshift/api#582: config/v1/types_proxy: Clarify trustedCA semantics
openshift/api#583: Clarify FROM behavior in builds
openshift/api#573: Add CRD generator documentation to Readme
openshift/api#576: Remove Description from CLI output to improve its display
openshift/api#589: Add missing enum validations
openshift/api#583: operator/ingress: add dnsrecord type
soltysh added a commit to soltysh/cluster-kube-controller-manager-operator that referenced this pull request Mar 6, 2020
In particular, to get

openshift/api#585: config: disable IPv6DualStack feature flag

so we don't launch kube-controller-manager with that (still-broken) feature

Also includes:

openshift/api#557: create the IBMCLoudPlatform type for the ingress operator try2
openshift/api#570: Clarify image config doc
openshift/api#569: Enable overriding service account issuer for bound tokens
openshift/api#527: Add kubebuilder annotations to the network types
openshift/api#574: add deprecaction notice for build pipeline strategy
openshift/api#582: config/v1/types_proxy: Clarify trustedCA semantics
openshift/api#583: Clarify FROM behavior in builds
openshift/api#573: Add CRD generator documentation to Readme
openshift/api#576: Remove Description from CLI output to improve its display
openshift/api#589: Add missing enum validations
openshift/api#583: operator/ingress: add dnsrecord type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants