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.

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

fix: remove conf server #10012

Merged
merged 37 commits into from
Sep 1, 2023
Merged

Conversation

kingluo
Copy link
Contributor

@kingluo kingluo commented Aug 13, 2023

Description

https://lists.apache.org/thread/b69vjkbdszdtk9y30k45c2tvg4f3hqwt

Fixes #9381, Fixes #8067, Fixes #9212, Fixes #9049, Fixes #9582, Fixes #9578

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)

@kingluo kingluo force-pushed the remove-conf-server branch 2 times, most recently from 3f900b9 to 9ef8686 Compare August 18, 2023 04:13
utils.SetRoute(e, httpexpect.Status5xx)
utils.GetRoute(eDataPanel, http.StatusOK)
utils.TestPrometheusEtcdMetric(ePrometheus, 0)

bandwidthAfter, durationAfter = utils.GetEgressBandwidthPerSecond(ePrometheus)
bpsAfter = bandwidthAfter / durationAfter

errorLog, err := utils.Log(apisixPod, cliSet.KubeCli, timeStart)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Skip this test because it fails because it's a conf server specific test.
Note that we must keep this test even if we do not execute it, because it shares some data with the next test.

Error: 023/08/19 08:19:53 [error] 46#46: *9103 [lua] exporter.lua:456: handler(): prometheus: failed to reach config server while processing metrics endpoint: has no healthy etcd endpoint available

@@ -91,12 +91,6 @@ if [ $count_test_access_log -eq 1 ]; then
exit 1
fi

count_access_log_off=`grep -c "access_log off;" conf/nginx.conf || true`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no access log for conf server anymore.

@@ -19,57 +19,6 @@

. ./t/cli/common.sh

echo '
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove useless schema tests

@@ -103,40 +45,6 @@ fi

echo "passed: control_plane should enable Admin API"

# use https
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no https between cp and conf server anymore

@@ -100,57 +100,6 @@ fi

echo "passed: could connect to etcd"

echo '
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only conf server requires all hosts to have the same scheme,


# The 'admin.apisix.dev' is injected by ci/common.sh@set_coredns

# etcd mTLS verify
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the mtls do not happen bewteen conf server and etcd anymore

@@ -168,7 +168,7 @@ make run
sleep 1
make stop

if ! grep -E 'upstream SSL certificate does not match \"127.0.0.1\" while SSL handshaking to upstream' logs/error.log; then
if ! grep -F 'certificate host mismatch' logs/error.log; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now https happens between lua-resty-http and etcd, and the log changes.

--- grep_error_log_out eval
qr/(peer closed connection in SSL handshake while SSL handshaking to upstream){1,}/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now https happens between lua-resty-http and etcd, and the log changes.

@@ -118,42 +118,7 @@ qr/(closed){1,}/



=== TEST 4: originate TLS connection to etcd cluster and verify TLS certificate (default behavior)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now we should make a CA bundle into the https://github.com/openresty/lua-nginx-module#lua_ssl_trusted_certificate directive in "/" location, instead of the conf server one.

@kingluo kingluo marked this pull request as ready for review August 22, 2023 08:22
leslie-tsang
leslie-tsang previously approved these changes Aug 29, 2023
@monkeyDluffy6017 monkeyDluffy6017 dismissed stale reviews from leslie-tsang and themself via ea56b73 September 1, 2023 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment