improve doc #64

dashanji · 2021-11-02T13:51:49Z

The PR is to improve the document to avoid repeated pitfalls. When using the infra e2e to test skywalking-swck, I encountered some problems and solved them.

Problem 1

When the controller's pod is ready, the controller's webhook may not be ready.
Solution
Use the command sleep 30s to wait for the webhook establishing.

Problem 2

If the interval is small, we may get the error.
Solution
Add a larger interval such as 30.

kezhenxu94 · 2021-11-02T13:55:04Z

docs/en/setup/Configuration-File.md

@@ -53,6 +53,8 @@ The `KinD` environment follow these steps:
 1. Wait until all steps are finished and all services are ready with the timeout(second).
 1. Expose all resource ports for host access.

+Notice, when you create resources such as webhooks, they need time to set up. If you get some error, you can try `sleep` to wait for the resources established. 


Is there any command kubectl that can wait for webhook ready?

I haven't found it yet.

kezhenxu94 · 2021-11-02T13:56:22Z

docs/en/setup/Configuration-File.md

@@ -150,6 +152,12 @@ trigger:

 The Trigger executed successfully at least once, after success, the next stage could be continued. Otherwise, there is an error and exit.

+If you get the following error message when using the trigger, You should check whether the port is useful. A possible reason may be that the exporting service needs time to set up, so you can add a larger interval to avoid the error.


Did you use the wait setting to wait for the resource ready?

Yes, the resouce is ready before port-forward, I can use curl to get data.

Does the e2e framework support NodePort or LoadBalancer? They are the more canonical ways to expose services than port-forward.

@kezhenxu94 How does the swctl connect to UI in current e2e cases?

We have settings to expose the ports in cluster

skywalking-infra-e2e/docs/en/setup/Configuration-File.md

Lines 43 to 46 in 5adf908

expose-ports: # Expose resource for host access

- namespace: # The resource namespace

resource: # The resource name, such as `pod/foo` or `service/foo`

port: # Want to expose port from resource

And you can reference via ${<resourceType>_<resourceName>_host}:${<resourceType>_<resourceName>_<port>}, such as ${service_skywalking_ui_host}:${service_skywalking_ui_80}

See example https://github.com/apache/skywalking/blob/7aa0ba70483b621d40452b0b2f53423f59f039e5/test/e2e-v2/cases/istio/als/e2e.yaml#L26-L29

Thanks @zhenxu

And FWIK, the item is not special to the e2e framework. The user should ensure a service accessible before accessing it. It's common sense more than a FAQ which the e2e user should care about.

wu-sheng · 2021-11-02T13:57:10Z

These added words seem like a FAQ. How about adding a FAQ section rather than this kind of notice or if? If there is no kubectl or wait policy for this kind of case.

If there are other ways, all these should be considered as a FAQ.

dashanji · 2021-11-02T14:09:34Z

These added words seem like a FAQ. How about adding a FAQ section rather than this kind of notice or if? If there is no kubectl or wait policy for this kind of case.

If there are other ways, all these should be considered as a FAQ.

Ok, I will add them to the FAQ section.

hanahmily

The health checker was introduced by kubernetes-sigs/controller-runtime#1588 which is not part of swck's dependencies though.

@dashanji Once swck upgrades to the latest controller-runtime, we could check the webhook server's health endpoint to ensure it. I hope it will happen in the next release.

wu-sheng · 2021-11-03T02:26:44Z

The health checker was introduced by kubernetes-sigs/controller-runtime#1588 which is not part of swck's dependencies though.

If this exists, then FAQ should not be added. Instead, this is a potential feature for SWCK.
Please confirm.

hanahmily · 2021-11-03T03:04:12Z

The health checker was introduced by kubernetes-sigs/controller-runtime#1588 which is not part of swck's dependencies though.

If this exists, then FAQ should not be added. Instead, this is a potential feature for SWCK. Please confirm.

Yep, to ensure webhook server before applying CRs is the e2e framework users' job. In another word, it's the behavior of kubernetes. If the e2e framework declared it's built on it, the user should accept this result. Based on that, this tip should be submitted to the kubernetes communication, not here 😄 .

dashanji · 2021-11-03T04:14:37Z

Yes，the two problems are to avoid users stepping on the same pit, how about adding them to the discussion's Q&A?

wu-sheng · 2021-11-06T12:30:20Z

Please be clear what bug it is, and how do you fix.

dashanji · 2021-11-06T12:31:01Z

When I use swctl --display yaml --base-url=http://${service_default_oap_host}:${service_default_oap_12800}/graphql endpoint list --keyword=hello --service-name Your_ApplicationName, get the follonging info.

- id: WW91cl9BcHBsaWNhdGlvbk5hbWU=.1_e0dFVH0vaGVsbG8=
  name: '{GET}/hello'

Then the error occurred like follows.
ERROR failed to unmarshal index: 0, yaml: line 2: did not find expected key

This bug is the yaml cannot recognize { and }. If we need to include { and }, the key or value needs to be included between '. I fix it in the last commit and delete the faq.

wu-sheng · 2021-11-06T12:33:42Z

Could you submit a new pull request? Which make sure the commit logs and PR content are clear.

As a new bug fix should be added in the 1.1.0 in https://github.com/apache/skywalking-infra-e2e/blob/main/CHANGES.md

dashanji · 2021-11-06T12:34:29Z

Could you submit a new pull request? Which make sure the commit logs and PR content are clear.

As a new bug fix should be added in the 1.1.0 in https://github.com/apache/skywalking-infra-e2e/blob/main/CHANGES.md

OK.

dashanji · 2021-11-06T14:14:40Z

Please be clear what bug it is, and how do you fix.

Sorry, this is not a bug, I forgot to add ' in the expected yaml when it contains {}.

improve doc

99d44e2

wu-sheng requested review from mrproliu, kezhenxu94 and hanahmily November 2, 2021 13:53

wu-sheng added this to the 1.1.0 milestone Nov 2, 2021

wu-sheng added the bug Something isn't working label Nov 2, 2021

kezhenxu94 reviewed Nov 2, 2021

View reviewed changes

add faq

6a7a219

hanahmily previously approved these changes Nov 3, 2021

View reviewed changes

dashanji dismissed hanahmily’s stale review via 8eb6b95 November 6, 2021 12:25

dashanji changed the title ~~improve doc~~ fix bug Nov 6, 2021

dashanji changed the title ~~fix bug~~ improve doc Nov 6, 2021

dashanji force-pushed the improve-doc branch from 8eb6b95 to 6a7a219 Compare November 6, 2021 12:54

wu-sheng closed this Nov 6, 2021

dashanji deleted the improve-doc branch November 6, 2021 12:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve doc #64

improve doc #64

dashanji commented Nov 2, 2021

kezhenxu94 Nov 2, 2021

dashanji Nov 2, 2021

kezhenxu94 Nov 2, 2021

dashanji Nov 2, 2021

hanahmily Nov 3, 2021

hanahmily Nov 3, 2021

kezhenxu94 Nov 3, 2021

hanahmily Nov 3, 2021

wu-sheng commented Nov 2, 2021 •

edited

Loading

dashanji commented Nov 2, 2021

hanahmily left a comment

wu-sheng commented Nov 3, 2021

hanahmily commented Nov 3, 2021

dashanji commented Nov 3, 2021 •

edited

Loading

wu-sheng commented Nov 6, 2021

dashanji commented Nov 6, 2021 •

edited

Loading

wu-sheng commented Nov 6, 2021

dashanji commented Nov 6, 2021

dashanji commented Nov 6, 2021

		@@ -150,6 +152,12 @@ trigger:

		The Trigger executed successfully at least once, after success, the next stage could be continued. Otherwise, there is an error and exit.

		If you get the following error message when using the trigger, You should check whether the port is useful. A possible reason may be that the exporting service needs time to set up, so you can add a larger interval to avoid the error.

	expose-ports: # Expose resource for host access
	- namespace: # The resource namespace
	resource: # The resource name, such as `pod/foo` or `service/foo`
	port: # Want to expose port from resource

improve doc #64

improve doc #64

Conversation

dashanji commented Nov 2, 2021

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

wu-sheng commented Nov 2, 2021 • edited Loading

dashanji commented Nov 2, 2021

hanahmily left a comment

Choose a reason for hiding this comment

wu-sheng commented Nov 3, 2021

hanahmily commented Nov 3, 2021

dashanji commented Nov 3, 2021 • edited Loading

wu-sheng commented Nov 6, 2021

dashanji commented Nov 6, 2021 • edited Loading

wu-sheng commented Nov 6, 2021

dashanji commented Nov 6, 2021

dashanji commented Nov 6, 2021

wu-sheng commented Nov 2, 2021 •

edited

Loading

dashanji commented Nov 3, 2021 •

edited

Loading

dashanji commented Nov 6, 2021 •

edited

Loading