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

[ci] Use 'go install' in Github Action setup #32735

Merged
merged 5 commits into from
Aug 24, 2022

Conversation

andrewkroh
Copy link
Member

What does this PR do?

As of Go 1.18, "go get no longer builds or installs packages in module-aware mode."

Why is it important?

All of the Github action build are failing.

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
  • I have made corresponding change to the default configuration files
  • 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.

As of Go 1.18, "go get no longer builds or installs packages in module-aware mode."
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Aug 19, 2022
@mergify
Copy link
Contributor

mergify bot commented Aug 19, 2022

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @andrewkroh? 🙏.
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

@elasticmachine
Copy link
Collaborator

elasticmachine commented Aug 19, 2022

💚 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: 2022-08-23T10:12:56.402+0000

  • Duration: 71 min 24 sec

Test stats 🧪

Test Results
Failed 0
Passed 22662
Skipped 1937
Total 24599

💚 Flaky test report

Tests succeeded.

🤖 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!)

@andrewkroh
Copy link
Member Author

This fixed the Go issue. But now exposes an issue with opentelemetry. The tests fail with:

2022-08-19T23:33:09.7728270Z File "/Users/runner/work/beats/beats/build/ve/darwin/lib/python3.10/site-packages/opentelemetry/context/init.py", line 131, in get_current
2022-08-19T23:33:09.7829630Z return _RUNTIME_CONTEXT.get_current() # type:ignore
2022-08-19T23:33:09.7830100Z tests/system/test_show_command.py sss [100%]>> python test: Unit Testing Complete
2022-08-19T23:33:09.7872720Z AttributeError: 'NoneType' object has no attribute 'get_current'

@kuisathaverat Could you please take a look at this failure. Any advice on how to fix this?

@v1v
Copy link
Member

v1v commented Aug 22, 2022

@andrewkroh , Ivan is on PTO.

https://github.com/elastic/beats/pull/29424/files was the original PR to enable the pytest-otel.

I'm trying to debug what's causing that particular failure. So far I can see python has been changed from 3.9 to 3.10

This action worked fine -> https://github.com/elastic/beats/runs/7614599682?check_suite_focus=true#step:7:59

============================= test session starts ==============================
platform darwin -- Python 3.9.13, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /Users/runner/work/beats/beats, configfile: pytest.ini
plugins: rerunfailures-9.1.1, timeout-1.4.2, otel-1.1.1

versus, this other one that failed
https://github.com/elastic/beats/runs/7927373354?check_suite_focus=true#step:7:103

============================= test session starts ==============================
platform darwin -- Python 3.10.6, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /Users/runner/work/beats/beats, configfile: pytest.ini
plugins: rerunfailures-9.1.1, timeout-1.4.2, otel-1.1.1

I wonder whether we could pin the version somehow to 3.9 and verify if it works

@v1v
Copy link
Member

v1v commented Aug 22, 2022

elastic/apm-pipeline-library#1687 contains further details, so I'll add the dependency as stated in open-telemetry/opentelemetry-python#2288 (comment)

@v1v
Copy link
Member

v1v commented Aug 22, 2022

I wonder if the requirements.txt files are read within the mage build system...

maybe make python-env is required to be executed beforehand? OTOH, mage unitTest should run the python tests too? Or just the Go tests? If the latter, then something is not right in the mage build system.

@andrewkroh , do you know who can assist on this?

@andrewkroh
Copy link
Member Author

I wonder if the requirements.txt files are read within the mage build system...

Yeah, it is used by mage when running the python tests. It creates the virtualenv before running the tests using it.

const (
libbeatRequirements = "{{ elastic_beats_dir}}/libbeat/tests/system/requirements.txt"
aixLibbeatRequirements = "{{ elastic_beats_dir}}/libbeat/tests/system/requirements_aix.txt"
)
var (
// VirtualenvReqs specifies a list of virtualenv requirements files to be
// used when calling PythonVirtualenv(). It defaults to the libbeat
// requirements.txt file.
VirtualenvReqs = []string{
libbeatRequirements,
}

I'm curious if the tests pass if we comment out the pytest-otel requirement.

@v1v
Copy link
Member

v1v commented Aug 23, 2022

Thanks for the analysis, I managed to reproduce the error locally after upgrading python to python 3.10, the suggested workaround does not work :/

I've just proposed 7dfa706 to force the python version to be 3.9. I guess, that approach could be good enough, as it does avoid upstream breaking changes by forcing what version to be used.

So far I see it works, given the action then the output matches with the python 3.9.13 version

>> python test: Unit Testing
============================= test session starts ==============================
platform darwin -- Python 3.9.13, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /Users/runner/work/beats/beats, configfile: pytest.ini
plugins: rerunfailures-9.1.1, timeout-1.4.2, otel-1.1.1

If it works, do you think we can keep the pytest-otel dependency for the time being until @kuisathaverat is back, so he can revisit this PR?

@andrewkroh
Copy link
Member Author

If it works, do you think we can keep the pytest-otel dependency for the time being until @kuisathaverat is back, so he can revisit this PR?

Yeah, if pinning to python 3.9 fixes it until we can fix the upstream otel packages, that sounds good.

@mergify
Copy link
Contributor

mergify bot commented Aug 24, 2022

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 feature/ci/use-go-install upstream/feature/ci/use-go-install
git merge upstream/main
git push upstream feature/ci/use-go-install

@andrewkroh andrewkroh marked this pull request as ready for review August 24, 2022 11:52
@andrewkroh andrewkroh requested a review from a team as a code owner August 24, 2022 11:52
@andrewkroh andrewkroh requested review from cmacknz and fearful-symmetry and removed request for a team August 24, 2022 11:52
@andrewkroh andrewkroh added the ci label Aug 24, 2022
@andrewkroh
Copy link
Member Author

This PR is working as expected. The remaining failure for "filebeat / macos" does not appear to be related to this change. I propose we merge this as is and continue monitoring these jobs for failures.

@andrewkroh andrewkroh added the Team:Automation Label for the Observability productivity team label Aug 24, 2022
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Aug 24, 2022
@andrewkroh andrewkroh requested a review from v1v August 24, 2022 12:12
@andrewkroh andrewkroh merged commit 6413e46 into elastic:main Aug 24, 2022
@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: 2022-08-24T11:52:08.390+0000

  • Duration: 84 min 39 sec

Test stats 🧪

Test Results
Failed 0
Passed 22664
Skipped 1937
Total 24601

💚 Flaky test report

Tests succeeded.

🤖 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!)

@rdner rdner added the backport-v7.17.0 Automated backport with mergify label Oct 14, 2022
mergify bot pushed a commit that referenced this pull request Oct 14, 2022
* Use 'go install' in Github Action setup

As of Go 1.18, "go get no longer builds or installs packages in module-aware mode."

* opentelemetry: pin dependencies for the otel

as stated in open-telemetry/opentelemetry-python#2288

* action: force python version 3.9

workaround the issue with open-telemetry/opentelemetry-python#2288

Co-authored-by: Victor Martinez <VictorMartinezRubio@gmail.com>
(cherry picked from commit 6413e46)

# Conflicts:
#	libbeat/tests/system/requirements_aix.txt
rdner pushed a commit that referenced this pull request Oct 14, 2022
* Use 'go install' in Github Action setup

As of Go 1.18, "go get no longer builds or installs packages in module-aware mode."

* opentelemetry: pin dependencies for the otel

as stated in open-telemetry/opentelemetry-python#2288

* action: force python version 3.9

workaround the issue with open-telemetry/opentelemetry-python#2288

Co-authored-by: Victor Martinez <VictorMartinezRubio@gmail.com>
(cherry picked from commit 6413e46)

# Conflicts:
#	libbeat/tests/system/requirements_aix.txt

Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>
chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023
* Use 'go install' in Github Action setup

As of Go 1.18, "go get no longer builds or installs packages in module-aware mode."

* opentelemetry: pin dependencies for the otel

as stated in open-telemetry/opentelemetry-python#2288

* action: force python version 3.9

workaround the issue with open-telemetry/opentelemetry-python#2288

Co-authored-by: Victor Martinez <VictorMartinezRubio@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.17.0 Automated backport with mergify ci Team:Automation Label for the Observability productivity team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants