Skip to content

fix: add debug yaml validation#7201

Merged
spacewander merged 4 commits into
apache:masterfrom
qizhendong1:debug-check-2
Jun 10, 2022
Merged

fix: add debug yaml validation#7201
spacewander merged 4 commits into
apache:masterfrom
qizhendong1:debug-check-2

Conversation

@qizhendong1

@qizhendong1 qizhendong1 commented Jun 6, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

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)

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

spacewander commented Jun 7, 2022

Copy link
Copy Markdown
Member

Please make the CI pass, thanks!

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

qizhendong1 commented Jun 7, 2022

Copy link
Copy Markdown
Contributor Author

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

@tokers

tokers commented Jun 8, 2022

Copy link
Copy Markdown
Contributor

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

I just re-ran them.

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

spacewander commented Jun 8, 2022

Copy link
Copy Markdown
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.

@qizhendong1

qizhendong1 commented Jun 8, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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)

@qizhendong1

qizhendong1 commented Jun 8, 2022

Copy link
Copy Markdown
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

tokers commented Jun 9, 2022

Copy link
Copy Markdown
Contributor

failed to validate debug config property "hook_conf" is required

The error message just explained the failure reason.

@qizhendong1

qizhendong1 commented Jun 9, 2022

Copy link
Copy Markdown
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
Comment thread apisix/debug.lua Outdated
Comment thread t/debug/hook.t Outdated
--- response_body
hello world
--- no_error_log
[error]

@spacewander spacewander Jun 9, 2022

Copy link
Copy Markdown
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.

@qizhendong1
qizhendong1 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
Comment thread t/debug/hook.t
hello world
--- error_log
read_debug_yaml(): failed to validate debug config property "hook_conf" is required
--- wait: 3

@spacewander spacewander Jun 10, 2022

Copy link
Copy Markdown
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)

@spacewander spacewander Jun 10, 2022

Copy link
Copy Markdown
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
@qizhendong1 @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
@qizhendong1 @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

@spacewander spacewander spacewander approved these changes
@tzssangglass tzssangglass tzssangglass 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

@qizhendong1 @spacewander @tokers @tzssangglass