fix: Some CRDs missing status sub-resource #1809

Chever-John · 2023-04-23T01:19:04Z

Type of change:

What this PR does / why we need it:

There are currently several resources in our custom resources that lack an observation configuration for the status sub-resource. And an e2e test file for this function. This PR aims to address these issues and completes the designs and corresponding e2e test files.

Closely linked to issue #1784.

Pre-submission checklist:

Did you explain what problem does this PR solve? Or what new features have been added?
Have you added corresponding test cases?
Have you modified the corresponding document?
Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

codecov-commenter · 2023-04-23T08:02:59Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (589bae6) 37.12% compared to head (ce7aeaf) 37.12%.

❗ Current head ce7aeaf differs from pull request most recent head eab049e. Consider uploading reports for the commit eab049e to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1809   +/-   ##
=======================================
  Coverage   37.12%   37.12%           
=======================================
  Files          93       93           
  Lines        7886     7886           
=======================================
  Hits         2928     2928           
  Misses       4568     4568           
  Partials      390      390

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Chever-John · 2023-05-04T10:44:42Z

test/e2e/suite-ingress/suite-ingress-features/status.go

+	ginkgo.It("check ApisixClusterConfig status is recorded", func() {
+		// create ApisixClusterConfig resource
+		clusterConfigName := "cluster-config-name"
+		assert.Nil(ginkgo.GinkgoT(), s.NewApisixClusterConfig(clusterConfigName, true, true), "create cluster config error")


Did u mean I lose e2e-test for this api(beta3)?

Thanks for pointing out!

No it is for apiV2. You are correct. I think you can undo it.
clusterConfigName := "cluster-config-name"

test/e2e/suite-ingress/suite-ingress-features/status.go

afzal442 · 2023-05-04T12:35:35Z

test/e2e/suite-ingress/suite-ingress-features/status.go

+
+		// status should be recorded as successfulen
+		output, err := s.GetOutputFromString("acc", clusterConfigName, "-o", "yaml")
+		assert.Nil(ginkgo.GinkgoT(), err, "Get output of ApisixPluginConfig resource")


Suggested change

assert.Nil(ginkgo.GinkgoT(), err, "Get output of ApisixPluginConfig resource")

assert.Nil(ginkgo.GinkgoT(), err, "Get output of ApisixClusterConfig resource")

tks, I will fix it!

afzal442 · 2023-05-07T10:33:49Z

test/e2e/suite-ingress/suite-ingress-features/status.go

@@ -275,6 +275,20 @@ wrw7im4TNSAdwVX4Y1F4svJ2as5SJn5QYGAzXDixNuwzXYrpP9rzA2s=
 		assert.Contains(ginkgo.GinkgoT(), output, `status: "False"`, "status.conditions.status  is recorded")
 	})

+	ginkgo.It("check ApisixClusterConfig status is recorded", func() {
+		// create ApisixClusterConfig resource
+		clusterConfigName := "apisix-cluster-config"


Hi, since you pass in "default" name to config, you can rmv this clusterConfigName := "apisix-cluster-config"

May I know the reason why I should remove this? I didn't pass in any other value for the config name besides 'apisix-cluster-config'. Is there anything else that I'm missing?

@afzal442

I apologize for overlooking the fact that the sample on the official website specifies the use of the default value as the default value for metadata.name.

No issues @Chever-John. It's alright. I am also learning with you. 😄

Thank you for your previous guidance, it helped me a lot. I was stuck creating a test environment before, but now I can work on this issue more efficiently:)

afzal442

Glad that you have achieved more than half way through this PR. 💪🏻 I can see CRD missing status sub-resource for ApisixClusterConfig is working but ApisixConsumer has some warnings now.

tao12345666333 · 2023-05-08T07:33:43Z

Please make CI happy first, thanks

Chever-John · 2023-05-15T16:58:24Z

@AlinsRan need re-run the ci

e2e-test ci should must be corrected in v2 when test suit-cluster.

tao12345666333 · 2023-06-06T04:09:38Z

test/e2e/suite-ingress/suite-ingress-features/status.go

@@ -234,8 +234,19 @@ wrw7im4TNSAdwVX4Y1F4svJ2as5SJn5QYGAzXDixNuwzXYrpP9rzA2s=
 	})

 	//TODO: ApisixGlobal
-	//TODO: ApisixConsumer  CRD missing status definition
-	//TODO: ApisixClusterConfig CRD missing status definition


Also need to add a test case for the status of ApisixClusterConfig.

github-actions · 2023-08-06T01:25:28Z

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 30 days if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.

tao12345666333 · 2023-08-31T02:22:30Z

could you please merge the latest code from master branch?

Chever-John · 2023-09-11T16:59:15Z

Sure. Pleased to do that~

Let me continue the previous work.

github-actions · 2023-11-12T01:26:23Z

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 30 days if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.

Revolyssup · 2023-11-21T07:51:24Z

@Chever-John There are merge conflicts. Are you still working on this?

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

Chever-John added 2 commits April 23, 2023 01:13

fix: Apisix Cluster Config status

132e9b2

test: e2e test for Apisix Cluster Config status

e4bcc97

Chever-John marked this pull request as draft April 23, 2023 01:22

chore: make update-all

2b31690

Chever-John marked this pull request as ready for review May 4, 2023 07:38

afzal442 suggested changes May 4, 2023

View reviewed changes

chore: type the cluster name

f9d8999

afzal442 suggested changes May 4, 2023

View reviewed changes

test/e2e/suite-ingress/suite-ingress-features/status.go Show resolved Hide resolved

afzal442 suggested changes May 4, 2023

View reviewed changes

Chever-John added 2 commits May 4, 2023 21:38

typo: from ApisixPluginConfig to ApisixClusterConfig

e9de4f1

fix: newApisixClusterConfig

73365ea

Chever-John marked this pull request as draft May 6, 2023 17:22

Chever-John added 2 commits May 7, 2023 01:37

fix: change clusterConfig's name

1d5d245

chore: change FIt to It

021fd8b

afzal442 suggested changes May 7, 2023

View reviewed changes

Chever-John added 2 commits May 7, 2023 22:08

fix: change config's name

d65a927

fix: add crd-subresource-status for apisixConsumer

21c6331

Chever-John requested a review from afzal442 May 7, 2023 17:10

Chever-John marked this pull request as ready for review May 7, 2023 17:12

afzal442 reviewed May 8, 2023

View reviewed changes

tao12345666333 added the bug Something isn't working label May 8, 2023

tao12345666333 requested review from tao12345666333 and AlinsRan May 8, 2023 07:33

Chever-John added 2 commits May 15, 2023 00:32

fix: yamllint error

e8784b9

typo: something need to correct

307298b

Chever-John added 2 commits May 16, 2023 23:22

fix: used resource must be released

a51ab89

fix: yamllint error

f59d8ef

Chever-John added 6 commits May 17, 2023 00:57

chore: fmt the status.go

a1e7e4c

chore: makefile

8b1dbe2

fix: package error

eb899b1

fix: remove unused package

f3c9496

fix: the ci error

dd922ab

fix: ci error

65aa00c

tao12345666333 added the area/controller label May 18, 2023

Chever-John added 3 commits May 19, 2023 00:41

fix: ci error

b1571f0

e2e-test ci should must be corrected in v2 when test suit-cluster.

fix: ci debug

6851347

fix: ci error

c0da9a8

tao12345666333 self-assigned this Jun 6, 2023

tao12345666333 reviewed Jun 6, 2023

View reviewed changes

github-actions bot added the stale label Aug 6, 2023

github-actions bot removed the stale label Sep 1, 2023

Chever-John added 2 commits September 12, 2023 01:08

Merge branch 'master' into fix/add-crd-status-field

7da1727

ci: Set cron for automatic execution upon changes in dev status

1e7c01d

github-actions bot added the stale label Nov 12, 2023

github-actions bot removed the stale label Nov 22, 2023

Revolyssup assigned Revolyssup and unassigned tao12345666333 Nov 29, 2023

solve merge conflicts

eab049e

Signed-off-by: Ashish Tiwari <ashishjaitiwari15112000@gmail.com>

Revolyssup approved these changes Dec 8, 2023

View reviewed changes

Sn0rt approved these changes Dec 11, 2023

View reviewed changes

AlinsRan approved these changes Dec 12, 2023

View reviewed changes

Revolyssup merged commit 1da1ca0 into apache:master Dec 13, 2023
79 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Some CRDs missing status sub-resource #1809

fix: Some CRDs missing status sub-resource #1809

Chever-John commented Apr 23, 2023 •

edited

Loading

codecov-commenter commented Apr 23, 2023 •

edited

Loading

This comment was marked as resolved.

Chever-John May 4, 2023

afzal442 May 4, 2023

Chever-John May 4, 2023

afzal442 May 4, 2023 •

edited

Loading

afzal442 May 4, 2023

Chever-John May 4, 2023 •

edited

Loading

afzal442 May 7, 2023

Chever-John May 7, 2023 •

edited

Loading

Chever-John May 7, 2023 •

edited

Loading

afzal442 May 8, 2023

Chever-John May 8, 2023

afzal442 left a comment

tao12345666333 commented May 8, 2023

Chever-John commented May 15, 2023

tao12345666333 Jun 6, 2023

github-actions bot commented Aug 6, 2023

tao12345666333 commented Aug 31, 2023

Chever-John commented Sep 11, 2023 •

edited

Loading

github-actions bot commented Nov 12, 2023

Revolyssup commented Nov 21, 2023

	assert.Nil(ginkgo.GinkgoT(), err, "Get output of ApisixPluginConfig resource")
	assert.Nil(ginkgo.GinkgoT(), err, "Get output of ApisixClusterConfig resource")

fix: Some CRDs missing status sub-resource #1809

fix: Some CRDs missing status sub-resource #1809

Conversation

Chever-John commented Apr 23, 2023 • edited Loading

Type of change:

What this PR does / why we need it:

Pre-submission checklist:

codecov-commenter commented Apr 23, 2023 • edited Loading

Codecov Report

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afzal442 May 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chever-John May 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chever-John May 7, 2023 • edited Loading

Choose a reason for hiding this comment

Chever-John May 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afzal442 left a comment

Choose a reason for hiding this comment

tao12345666333 commented May 8, 2023

Chever-John commented May 15, 2023

Choose a reason for hiding this comment

github-actions bot commented Aug 6, 2023

tao12345666333 commented Aug 31, 2023

Chever-John commented Sep 11, 2023 • edited Loading

github-actions bot commented Nov 12, 2023

Revolyssup commented Nov 21, 2023

Chever-John commented Apr 23, 2023 •

edited

Loading

codecov-commenter commented Apr 23, 2023 •

edited

Loading

afzal442 May 4, 2023 •

edited

Loading

Chever-John May 4, 2023 •

edited

Loading

Chever-John May 7, 2023 •

edited

Loading

Chever-John May 7, 2023 •

edited

Loading

Chever-John commented Sep 11, 2023 •

edited

Loading