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

Make test output honor output.elasticsearch.proxy_url #36715

Closed

Conversation

sakurai-youhei
Copy link
Member

@sakurai-youhei sakurai-youhei commented Oct 2, 2023

This PR will fix the part of #24751 related to Elasticsearch. The code here depends on elastic/elastic-agent-libs#154.

$ ./filebeat test output -E output.elasticsearch.username=admin -E output.elasticsearch.password=testing -E output.elasticsearch.hosts=http://localhost:9200 -E output.elasticsearch.proxy_url=http://proxy:testing@localhost:3128
elasticsearch: http://localhost:9200...
  parse url... OK
  proxy...
    parse host... OK
    dns lookup... OK
    addresses: 127.0.0.1
    dial up... OK
  connection...
    proxy... OK
    dial up... OK
  TLS... WARN secure connection disabled
  talk to server... OK
  version: 8.11.0-SNAPSHOT

Proposed commit message

See title

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation - no change is needed.
  • I have made corresponding change to the default configuration files - no change is needed.
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.
hidden because this section is no longer valid ## Author's Checklist

How to test this PR locally

sakurai@wsl-debian:~/go/src/github.com/elastic/beats/libbeat$ go test -v ./tests/integration... -tags integration -run TestCmdTestOutput
=== RUN   TestCmdTestOutput
--- PASS: TestCmdTestOutput (0.34s)
=== RUN   TestCmdTestOutputBadHost
--- PASS: TestCmdTestOutputBadHost (2.29s)
=== RUN   TestCmdTestOutputProxy
--- PASS: TestCmdTestOutputProxy (0.45s)
=== RUN   TestCmdTestOutputProxyBadHost
--- PASS: TestCmdTestOutputProxyBadHost (0.34s)
=== RUN   TestCmdTestOutputBadProxy
--- PASS: TestCmdTestOutputBadProxy (2.28s)
=== RUN   TestCmdTestOutputBadProxyDisable
--- PASS: TestCmdTestOutputBadProxyDisable (0.35s)
PASS
ok      github.com/elastic/beats/v7/libbeat/tests/integration   6.046s

Related issues

Use cases

I frequently rely on test output command for troubleshooting, so its reliability is key.

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Oct 2, 2023
@mergify
Copy link
Contributor

mergify bot commented Oct 2, 2023

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @sakurai-youhei? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

sakurai-youhei added a commit to sakurai-youhei/elastic-agent-libs that referenced this pull request Oct 2, 2023
@sakurai-youhei sakurai-youhei added libbeat backport-skip Skip notification from the automated backport with mergify labels Oct 2, 2023
@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 2, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-11-01T23:16:48.418+0000

  • Duration: 134 min 33 sec

Test stats 🧪

Test Results
Failed 0
Passed 28627
Skipped 2015
Total 30642

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@sakurai-youhei
Copy link
Member Author

sakurai-youhei commented Oct 4, 2023

I will complete remaining works in this PR once the depending elastic/elastic-agent-libs#154 will have been merged.

@sakurai-youhei sakurai-youhei marked this pull request as ready for review October 24, 2023 15:55
@sakurai-youhei sakurai-youhei requested a review from a team as a code owner October 24, 2023 15:55
@pierrehilbert pierrehilbert added the Team:Elastic-Agent Label for the Agent team label Oct 25, 2023
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Oct 25, 2023
libbeat/esleg/eslegclient/connection.go Outdated Show resolved Hide resolved
libbeat/esleg/eslegclient/connection.go Outdated Show resolved Hide resolved
libbeat/esleg/eslegclient/connection.go Outdated Show resolved Hide resolved
Comment on lines 408 to 411
if proxyURL := conn.Transport.Proxy.URL; proxyURL == nil || conn.Transport.Proxy.Disable {
dialer = transport.TestNetDialer(d, conn.Transport.Timeout)
} else {
dialer = transport.NetDialer(conn.Transport.Timeout)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm trying to understand the logic here. Why is the choice between the TestNetDialer and NetDialer is based on the proxy URL/proxy being enabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

@belimawr That's because the TCP connectivity has already been examined by TestNetDialer if the peer is the proxy server. Thanks to your comment, I realize there's much room to improve readability in the Test function. I hope the new one is self-explanatory enough.

Copy link
Contributor

mergify bot commented Nov 1, 2023

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b honor-elasticsearch-proxy_url upstream/honor-elasticsearch-proxy_url
git merge upstream/main
git push upstream honor-elasticsearch-proxy_url

Copy link
Contributor

mergify bot commented Nov 30, 2023

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b honor-elasticsearch-proxy_url upstream/honor-elasticsearch-proxy_url
git merge upstream/main
git push upstream honor-elasticsearch-proxy_url

@elasticmachine
Copy link
Collaborator

💔 Tests Failed

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-12-05T03:10:23.802+0000

  • Duration: 144 min 30 sec

Test stats 🧪

Test Results
Failed 1
Passed 28716
Skipped 2015
Total 30732

Test errors 1

Expand to view the tests failures

Build&Test / filebeat-pythonIntegTest / test_shutdown – filebeat.tests.system.test_shutdown.Test
    Expand to view the error details

     AssertionError: Expected exit code to be 0, but it was -15 
    

    Expand to view the stacktrace

     self = <test_shutdown.Test testMethod=test_shutdown>
    
        @unittest.skipIf(platform.platform().startswith("Windows-7"),
                         "Flaky test: https://github.com/elastic/beats/issues/22795")
        def test_shutdown(self):
            """
            Test starting and stopping Filebeat under load.
            """
        
            self.nasa_logs()
        
            self.render_config_template(
                path=os.path.abspath(self.working_dir) + "/log/*",
                ignore_older="1h"
            )
            for i in range(1, 5):
                proc = self.start_beat(logging_args=["-e", "-v"])
                time.sleep(.5)
    >           proc.check_kill_and_wait()
    
    tests/system/test_shutdown.py:31: 
    _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
    ../libbeat/tests/system/beat/beat.py:153: in check_kill_and_wait
        return self.check_wait(exit_code=exit_code)
    _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
    
    self = <beat.beat.Proc object at 0x7f51ddd5ab30>, exit_code = 0
    
        def check_wait(self, exit_code=0):
            """
            check_wait waits for the process to exit, and checks the return code of the process
            """
            actual_exit_code = self.wait()
    >       assert actual_exit_code == exit_code, f"Expected exit code to be {exit_code}, but it was {actual_exit_code}"
    E       AssertionError: Expected exit code to be 0, but it was -15
    
    ../libbeat/tests/system/beat/beat.py:133: AssertionError 
    

Steps errors 4

Expand to view the steps failures

filebeat-pythonIntegTest - mage pythonIntegTest
  • Took 41 min 16 sec . View more details here
  • Description: mage pythonIntegTest
filebeat-pythonIntegTest - mage pythonIntegTest
  • Took 38 min 58 sec . View more details here
  • Description: mage pythonIntegTest
filebeat-pythonIntegTest - mage pythonIntegTest
  • Took 39 min 44 sec . View more details here
  • Description: mage pythonIntegTest
Error signal
  • Took 0 min 0 sec . View more details here
  • Description: Error 'hudson.AbortException: script returned exit code 1'

🐛 Flaky test report

❕ There are test failures but not known flaky tests.

Expand to view the summary

Genuine test errors 1

💔 There are test failures but not known flaky tests, most likely a genuine test failure.

  • Name: Build&Test / filebeat-pythonIntegTest / test_shutdown – filebeat.tests.system.test_shutdown.Test

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

Copy link
Contributor

mergify bot commented Dec 7, 2023

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b honor-elasticsearch-proxy_url upstream/honor-elasticsearch-proxy_url
git merge upstream/main
git push upstream honor-elasticsearch-proxy_url

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-12-08T06:42:07.756+0000

  • Duration: 131 min 20 sec

Test stats 🧪

Test Results
Failed 0
Passed 28725
Skipped 2015
Total 30740

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

Copy link
Contributor

mergify bot commented Dec 12, 2023

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b honor-elasticsearch-proxy_url upstream/honor-elasticsearch-proxy_url
git merge upstream/main
git push upstream honor-elasticsearch-proxy_url

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-12-17T01:10:24.275+0000

  • Duration: 135 min 57 sec

Test stats 🧪

Test Results
Failed 0
Passed 28737
Skipped 2015
Total 30752

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@sakurai-youhei
Copy link
Member Author

I'm closing this PR to clean up my PR list. Please feel free to reopen PR and/or ping me anytime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip Skip notification from the automated backport with mergify bug libbeat Team:Elastic-Agent Label for the Agent team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Command filebeat test output does not honor proxy_url setting for output.logstash or output.elasticsearch
5 participants