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

fix: add debug yaml validation #7201

Merged
merged 4 commits into from
Jun 10, 2022
Merged

fix: add debug yaml validation #7201

merged 4 commits into from
Jun 10, 2022

Conversation

zhendongcmss
Copy link
Contributor

@zhendongcmss zhendongcmss commented Jun 6, 2022 •
edited
Loading

Description

Fixes # (issue) #7159

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@zhendongcmss zhendongcmss force-pushed the debug-check-2 branch 2 times, most recently from c48a0ab to 31a66b6 Compare June 7, 2022 01:17
@zhendongcmss zhendongcmss marked this pull request as ready for review June 7, 2022 01:18
@spacewander
Copy link
Member

Please make the CI pass, thanks!

@zhendongcmss zhendongcmss force-pushed the debug-check-2 branch 2 times, most recently from b7802ec to ca91eba Compare June 7, 2022 06:49
@zhendongcmss
Copy link
Contributor Author

@spacewander The CI test is not stable, is there a way to restart the CI test without git push?

@tokers
Copy link
Contributor

tokers commented Jun 8, 2022

@spacewander The CI test is not stable, is there a way to restart the CI test without git push?

I just re-ran them.

@zhendongcmss zhendongcmss force-pushed the debug-check-2 branch 2 times, most recently from 6a7c1f6 to cac84cc Compare June 8, 2022 01:17
@spacewander
Copy link
Member

https://github.com/apache/apisix/runs/6785395449?check_suite_focus=true

init_worker_by_lua error: /home/runner/work/apisix/apisix/apisix/debug.lua:225: attempt to index local 'hook_conf' (a nil value)"

It seems this causes the CI failure.

@zhendongcmss
Copy link
Contributor Author

zhendongcmss commented Jun 8, 2022 •
edited
Loading

I got the eror, is there way to let it pass ?

https://github.com/apache/apisix/runs/6790365518?check_suite_focus=true#step:15:704

...
--- error_log
read_debug_yaml(): failed to validate debug config property "hook_conf" is required

Error: led test 't/debug/hook.t TEST 5: missing hook_conf - pattern "[error]" should not match any line in error.log but matches line "2022/06/08 09:45:54 [error] 18297#18297: *2 [lua] debug.lua:147: read_debug_yaml(): failed to validate debug config property \"hook_conf\" is required, context: init_worker_by_lua*" (req 0)

@zhendongcmss
Copy link
Contributor Author

@spacewander The CI test is not stable, is there a way to restart the CI test without git push?

I just re-ran them.

How to re-run ? I don't found the re-run button.

@tokers
Copy link
Contributor

tokers commented Jun 9, 2022

failed to validate debug config property "hook_conf" is required

The error message just explained the failure reason.

@zhendongcmss
Copy link
Contributor Author

failed to validate debug config property "hook_conf" is required

The error message just explained the failure reason.

Actually, them match, why test case doesn't pass ?

spacewander
spacewander reviewed Jun 9, 2022
View reviewed changes
apisix/debug.lua Outdated Show resolved Hide resolved
t/debug/hook.t Outdated
--- response_body
hello world
--- no_error_log
[error]
Copy link
Member

Choose a reason for hiding this comment

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

We can't check there is no [error] because failed to validate debug config property "hook_conf" is required is an error level log.

@zhendongcmss zhendongcmss force-pushed the debug-check-2 branch 2 times, most recently from 0b916bd to ee99e0d Compare June 9, 2022 07:45
spacewander
spacewander reviewed Jun 10, 2022
View reviewed changes
t/debug/hook.t
hello world
--- error_log
read_debug_yaml(): failed to validate debug config property "hook_conf" is required
--- wait: 3
Copy link
Member

Choose a reason for hiding this comment

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

I think wait: 2 is enough as we load debug yaml per second:

apisix/apisix/debug.lua

Line 311 in f2326af

ngx.timer.every(1, sync_debug_status)

Copy link
Member

Choose a reason for hiding this comment

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

Anyway, wait for one more seconds is acceptable

@spacewander spacewander merged commit f0f5b48 into apache:master Jun 10, 2022
hongbinhsu pushed a commit to fitphp/apix that referenced this pull request Jun 16, 2022
Merge branch 'upstream/master' into github/master
4ed81b5 
* upstream/master: (46 commits)
  docs: fix err in batch-processor (apache#7259)
  docs(deployment): sync design to online docs (apache#7256)
  feat(deployment): add structure of traditional role (apache#7249)
  fix(benchmark): write worker_processes into config.yaml (apache#7250)
  docs: correct the repo url (apache#7253)
  feat: Add support for capturing OIDC refresh tokens (apache#7220)
  feat(ssl): support get upstream cert from ssl object (apache#7221)
  chore: validate etcd conf strictly (apache#7245)
  fix(api-response): check response header format (apache#7238)
  fix: duplicate X-Forwarded-Proto will be sent as string (apache#7229)
  fix: distinguish different upstreams even they have the same addr (apache#7213)
  docs: make company on README more preciser (apache#7230)
  test: remove unused required etcd (apache#7225)
  fix: add debug yaml validation (apache#7201)
  change: remove upstream.enable_websocket which is deprecated since 2020 (apache#7222)
  docs: add re case on response-rewrite plugin (apache#7197)
  docs: add API Gateway keyword and AWS graviton3. (apache#7217)
  fix(response-rewrite): schema format error (apache#7212)
  docs(proxy-rewrite): remove empty space (apache#7210)
  chore: require http_stub_status_module exists (apache#7208)
  ...
spacewander pushed a commit that referenced this pull request Jun 30, 2022
@zhendongcmss @spacewander
fix: add debug yaml validation (#7201)
273f608
Liu-Junlin pushed a commit to Liu-Junlin/apisix that referenced this pull request Nov 4, 2022
@zhendongcmss @Liu-Junlin
fix: add debug yaml validation (apache#7201)
c0473ac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tzssangglass tzssangglass tzssangglass approved these changes

@spacewander spacewander spacewander approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zhendongcmss @spacewander @tokers @tzssangglass