Add openAPI schema for AntreaAgentInfo and AntreaControllerInfo #5206

ceclinux · 2023-07-05T07:02:11Z

No description provided.

luolanzone · 2023-07-11T02:51:50Z

docs/api.md

+| `AntreaAgentInfo` | v1 | v1.0.0 | N/A | N/A |
+| `AntreaControllerInfo` | v1 | v1.0.0 | N/A | N/A |


Suggested change

| `AntreaAgentInfo` | v1 | v1.0.0 | N/A | N/A |

| `AntreaControllerInfo` | v1 | v1.0.0 | N/A | N/A |

| `AntreaAgentInfo` | v1 | v1.13.0 | N/A | N/A |

| `AntreaControllerInfo` | v1 | v1.13.0 | N/A | N/A |

luolanzone · 2023-07-11T02:52:55Z

docs/api.md

+| `AntreaAgentInfo` | v1beta1 | v1.0.0 | v1.13.0 | v2.0.0 |
+| `AntreaControllerInfo` | v1beta1 | v1.0.0 | v1.13.0 | v2.0.0 |


Suggested change

| `AntreaAgentInfo` | v1beta1 | v1.0.0 | v1.13.0 | v2.0.0 |

| `AntreaControllerInfo` | v1beta1 | v1.0.0 | v1.13.0 | v2.0.0 |

| `AntreaAgentInfo` | v1beta1 | v1.0.0 | v1.13.0 | N/A |

| `AntreaControllerInfo` | v1beta1 | v1.0.0 | v1.13.0 | N/A |

We are probably unable to remove them in 2.0.0 considering we need to follow the K8s deprecation policy.

luolanzone · 2023-07-11T02:54:40Z

hack/update-codegen-dockerized.sh

@@ -146,7 +150,7 @@ function generate_antrea_client_code {
    --input-dirs "${ANTREA_PKG}/pkg/apis/controlplane/v1beta2" \
    --input-dirs "${ANTREA_PKG}/pkg/apis/system/v1beta1" \
    --input-dirs "${ANTREA_PKG}/pkg/apis/stats/v1alpha1" \
-    --input-dirs "${ANTREA_PKG}/pkg/apis/crd/v1beta1" \


I think this shouldn't be removed since we still have v1beta1 crd.

luolanzone · 2023-07-11T02:56:47Z

pkg/apis/crd/v1/doc.go

@@ -0,0 +1,20 @@
+// Copyright 2019 Antrea Authors


Suggested change

// Copyright 2019 Antrea Authors

// Copyright 2023 Antrea Authors

luolanzone · 2023-07-11T02:56:54Z

pkg/apis/crd/v1/register.go

@@ -0,0 +1,56 @@
+// Copyright 2019 Antrea Authors


luolanzone · 2023-07-11T03:01:48Z

pkg/apis/system/v1beta1/register.go

@tnqn should this api be promoted to v1 as controller/agent info?

luolanzone · 2023-07-11T03:02:25Z

test/e2e/framework.go

@@ -2880,8 +2880,8 @@ func retryOnConnectionLostError(backoff wait.Backoff, fn func() error) error {
 }

 func (data *TestData) checkAntreaAgentInfo(interval time.Duration, timeout time.Duration, name string) error {
-	err := wait.PollImmediate(interval, timeout, func() (bool, error) {
-		aai, err := data.crdClient.CrdV1beta1().AntreaAgentInfos().Get(context.TODO(), name, metav1.GetOptions{})
+	err := wait.Poll(interval, timeout, func() (bool, error) {


Why is the method changed?

antoninbas · 2023-07-17T19:00:32Z

build/charts/antrea/crds/antreaagentinfo.yaml

+      schema:
+        openAPIV3Schema:
+          type: object
+          x-kubernetes-preserve-unknown-fields: true


When we discussed this API promotion, we said we would add a correct openAPI schema as part of the CRD definition. This PR does not do that. See #5068

antoninbas · 2023-07-17T19:02:04Z

@jianjuns @tnqn just confirming that you are ok with promoting AntreaAgentInfo / AntreaControllerInfo to v1 (instead of another beta version, e.g., v1beta2)?

jianjuns · 2023-07-17T20:14:06Z

I feel more comfortable with beta, but no strong opinion.

tnqn · 2023-07-18T02:28:37Z

I don't have strong opinion either. If we don't really have a strong reason to do it, should we just enhance the validation in this release? It seems fine to me to just enhance the schema in-place?

antoninbas · 2023-07-18T04:40:15Z

I don't have strong opinion either. If we don't really have a strong reason to do it, should we just enhance the validation in this release? It seems fine to me to just enhance the schema in-place?

I'm fine with staying in beta and adding the correct openAPI schema. I feel like even if we only add the schema, it is better to increment the version number (v1beta1 -> v1beta2). But then again, this is essentially a "read-only" API, so it probably doesn't matter very much.

ceclinux · 2023-07-18T08:35:15Z

I will update this pr with two modifications:

promoting AntreaAgentInfo / AntreaControllerInfo to v1beta2 instead of v1
add a correct openAPI schema
Please leave more comments if you have other ideas.Thanks @antoninbas @jianjuns @tnqn

tnqn · 2023-07-18T11:10:11Z

Introducing a new version still comes with some cost: all clients including antrea-ui and nephe need to update their code, but there is no usage change, and no stage change like other API promotions (from alpha to beta). Given the enhancement doesn't break anything and there is no real benefit from v1beta2, should we just keep v1beta1?

antoninbas · 2023-07-18T20:26:16Z

Introducing a new version still comes with some cost: all clients including antrea-ui and nephe need to update their code, but there is no usage change, and no stage change like other API promotions (from alpha to beta). Given the enhancement doesn't break anything and there is no real benefit from v1beta2, should we just keep v1beta1?

Given this API usage (read-only for clients), I am fine with this

ceclinux · 2023-07-20T12:50:43Z

/test-multicast-e2e

ceclinux · 2023-07-20T12:50:54Z

/test-multicast-e2e

antoninbas · 2023-07-20T17:01:03Z

build/charts/antrea/crds/antreaagentinfo.yaml

+              items:
+                type: string
+            apiCABundle:
+              type: string


I believe this should also have a format: byte property, given that the Go type is []byte and it will be base64-encoded when it is serialized.

see https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/

antoninbas · 2023-07-20T17:04:48Z

build/charts/antrea/crds/antreaagentinfo.yaml

+                  type: string
+                flowTable:
+                  type: object
+                  x-kubernetes-preserve-unknown-fields: true


would this work here instead:

flowTable: type: object additionalProperties: type: number format: int32

updated. Number represents floating point numbers. I think it should be type: integer?

antoninbas · 2023-07-20T17:05:35Z

build/charts/antrea/crds/antreaagentinfo.yaml

+                networkPolicyNum:
+                  type: integer
+                addressGroupNum:
+                  type: integer
+                appliedToGroupNum:
+                  type: integer


add format: in32 for these?

antoninbas

one more comment, otherwise lgtm

antoninbas · 2023-07-21T04:00:49Z

build/charts/antrea/crds/antreacontrollerinfo.yaml

+                networkPolicyNum:
+                  type: integer
+                addressGroupNum:
+                  type: integer
+                appliedToGroupNum:
+                  type: integer


these are missing format: int32

updated. Thanks

Signed-off-by: ceclinux <[email protected]>

antoninbas

LGTM

antoninbas · 2023-07-21T17:44:56Z

/test-all

ceclinux mentioned this pull request Jul 4, 2023

[Antrea 2.0] Promote APIs to appropriate versions and feature gates to appropriate stages #5068

Closed

27 tasks

ceclinux force-pushed the upgrade-aci-and-aai-api branch from 0ba6da0 to 36df4cf Compare July 5, 2023 07:36

luolanzone added api-review Categorizes an issue or PR as actively needing an API review. action/release-note Indicates a PR that should be included in release notes. labels Jul 5, 2023

luolanzone added this to the Antrea v1.13 release milestone Jul 5, 2023

ceclinux force-pushed the upgrade-aci-and-aai-api branch 6 times, most recently from 68b647b to 656a58b Compare July 6, 2023 06:52

luolanzone reviewed Jul 11, 2023

View reviewed changes

ceclinux force-pushed the upgrade-aci-and-aai-api branch 2 times, most recently from f93c864 to a140121 Compare July 12, 2023 09:51

antoninbas requested changes Jul 17, 2023

View reviewed changes

ceclinux force-pushed the upgrade-aci-and-aai-api branch from a140121 to d5a7e3f Compare July 20, 2023 11:31

ceclinux changed the title ~~Promote AntreaAgentInfo and AntreaControllerInfo API to v1.~~ Add openAPI schema for AntreaAgentInfo and AntreaControllerInfo Jul 20, 2023

ceclinux changed the title ~~Add openAPI schema for AntreaAgentInfo and AntreaControllerInfo~~ [WIP]Add openAPI schema for AntreaAgentInfo and AntreaControllerInfo Jul 20, 2023

ceclinux force-pushed the upgrade-aci-and-aai-api branch 2 times, most recently from bd886e9 to 62ba983 Compare July 20, 2023 14:18

ceclinux changed the title ~~[WIP]Add openAPI schema for AntreaAgentInfo and AntreaControllerInfo~~ Add openAPI schema for AntreaAgentInfo and AntreaControllerInfo Jul 20, 2023

antoninbas reviewed Jul 20, 2023

View reviewed changes

ceclinux force-pushed the upgrade-aci-and-aai-api branch from 62ba983 to 4ca2b30 Compare July 21, 2023 02:59

antoninbas reviewed Jul 21, 2023

View reviewed changes

Add openAPI schema for AntreaAgentInfo and AntreaControllerInfo

32ccd8d

Signed-off-by: ceclinux <[email protected]>

ceclinux force-pushed the upgrade-aci-and-aai-api branch from 4ca2b30 to 32ccd8d Compare July 21, 2023 08:32

antoninbas approved these changes Jul 21, 2023

View reviewed changes

antoninbas merged commit 396cbd6 into antrea-io:main Jul 21, 2023

ceclinux deleted the upgrade-aci-and-aai-api branch July 25, 2023 00:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add openAPI schema for AntreaAgentInfo and AntreaControllerInfo #5206

Add openAPI schema for AntreaAgentInfo and AntreaControllerInfo #5206

ceclinux commented Jul 5, 2023

luolanzone Jul 11, 2023

ceclinux Jul 12, 2023

luolanzone Jul 11, 2023

ceclinux Jul 12, 2023

luolanzone Jul 11, 2023

ceclinux Jul 12, 2023

luolanzone Jul 11, 2023

ceclinux Jul 12, 2023

luolanzone Jul 11, 2023

ceclinux Jul 12, 2023

luolanzone Jul 11, 2023

luolanzone Jul 11, 2023

ceclinux Jul 12, 2023

antoninbas Jul 17, 2023

antoninbas commented Jul 17, 2023 •

edited

Loading

jianjuns commented Jul 17, 2023

tnqn commented Jul 18, 2023

antoninbas commented Jul 18, 2023

ceclinux commented Jul 18, 2023

tnqn commented Jul 18, 2023

antoninbas commented Jul 18, 2023

ceclinux commented Jul 20, 2023

ceclinux commented Jul 20, 2023

antoninbas Jul 20, 2023

ceclinux Jul 21, 2023

antoninbas Jul 20, 2023

ceclinux Jul 21, 2023

antoninbas Jul 20, 2023

ceclinux Jul 21, 2023

antoninbas left a comment

antoninbas Jul 21, 2023

ceclinux Jul 21, 2023

antoninbas left a comment

antoninbas commented Jul 21, 2023

		\| `AntreaAgentInfo` \| v1 \| v1.0.0 \| N/A \| N/A \|
		\| `AntreaControllerInfo` \| v1 \| v1.0.0 \| N/A \| N/A \|

		\| `AntreaAgentInfo` \| v1beta1 \| v1.0.0 \| v1.13.0 \| v2.0.0 \|
		\| `AntreaControllerInfo` \| v1beta1 \| v1.0.0 \| v1.13.0 \| v2.0.0 \|

	// Copyright 2019 Antrea Authors
	// Copyright 2023 Antrea Authors

Add openAPI schema for AntreaAgentInfo and AntreaControllerInfo #5206

Add openAPI schema for AntreaAgentInfo and AntreaControllerInfo #5206

Conversation

ceclinux commented Jul 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoninbas commented Jul 17, 2023 • edited Loading

jianjuns commented Jul 17, 2023

tnqn commented Jul 18, 2023

antoninbas commented Jul 18, 2023

ceclinux commented Jul 18, 2023

tnqn commented Jul 18, 2023

antoninbas commented Jul 18, 2023

ceclinux commented Jul 20, 2023

ceclinux commented Jul 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoninbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoninbas left a comment

Choose a reason for hiding this comment

antoninbas commented Jul 21, 2023

antoninbas commented Jul 17, 2023 •

edited

Loading