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

Elastic Defend arm64 is not supported on Windows yet #4155

Merged
merged 35 commits into from
Feb 13, 2024

Conversation

intxgo
Copy link
Contributor

@intxgo intxgo commented Jan 27, 2024

What does this PR do?

This PR enhances elastic-endpoint spec to gracefully refrain from ever installing elastic-endpoint on Windows ARM, because Elastic Defend is not supported on it.

The win32 API used IsWow64Process2 is available only on Windows >= Win10, however Agent is currently compiled using Go 1.21 which requires at least Windows 10 or Server 2016.

The native architecture constants can be found here on MSDN web, however I believe it's unimportant to port them all since Elastic Defend is compiled only for x86_64 for Windows.

Reading environment variables could be an alternative solution, as Windows sets them by default, examples:

PROCESSOR_ARCHITECTURE=ARM64
PROCESSOR_IDENTIFIER=ARMv8 (64-bit) Family 8 Model 0 Revision   0, QEMU
PROCESSOR_ARCHITECTURE=AMD64
PROCESSOR_IDENTIFIER=Intel64 Family 6 Model 151 Stepping 2, GenuineIntel
PROCESSOR_ARCHITECTURE=AMD64
PROCESSOR_IDENTIFIER=AMD64 Family 25 Model 97 Stepping 2, AuthenticAMD

However environment variables could be easily modified, therefore calling native API is better solution.

Why is it important?

Elastic Defend is not provided yet for Windows ARM (Microsoft Surface X or VM on Apple M1/M2/M3) but since Windows Elastic Agent amd64 works fine on Windows ARM in emulation people install it.

Handling gracefully the incompatibility enables installing Agent across a fleet of Windows AMD64 and ARM64 devices with the same policy.

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/fragments using the changelog tool
  • I have added an integration test or an E2E test

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

Questions to ask yourself

  • How are we going to support this in production?
  • How are we going to measure its adoption?
  • How are we going to debug this?
  • What are the metrics I should take care of?
  • ...

@intxgo intxgo added the enhancement New feature or request label Jan 27, 2024
@mergify mergify bot assigned intxgo Jan 27, 2024
Copy link
Contributor

mergify bot commented Jan 27, 2024

This pull request does not have a backport label. Could you fix it @intxgo? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

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

NOTE: backport-skip has been added to this pull request.

@intxgo intxgo marked this pull request as ready for review January 27, 2024 00:09
@intxgo intxgo requested a review from a team as a code owner January 27, 2024 00:09
@intxgo intxgo requested review from blakerouse and pchila January 27, 2024 00:09
@pierrehilbert pierrehilbert added the Team:Elastic-Agent Label for the Agent team label Jan 27, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

Copy link

@nfritts nfritts left a comment

Choose a reason for hiding this comment

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

This looks good to me from Endpoints perspective

Copy link
Member

@pchila pchila left a comment

Choose a reason for hiding this comment

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

Aside from the small nitpick, the change looks good to me.
There is the small issue of the error in the endpoint spec though (CI is failing because of that)

@intxgo intxgo marked this pull request as draft January 29, 2024 16:49
@intxgo
Copy link
Contributor Author

intxgo commented Feb 12, 2024

could you review this again since the implementation has changed significantly

@michalpristas @pchila

@intxgo intxgo marked this pull request as ready for review February 12, 2024 17:16
@intxgo intxgo enabled auto-merge (squash) February 13, 2024 08:13
@intxgo intxgo disabled auto-merge February 13, 2024 08:17
Copy link
Contributor

@michalpristas michalpristas left a comment

Choose a reason for hiding this comment

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

i think it looks good, fix from sysinfo is included for older windows versions as well.
haven't tested this on arm machine

Copy link

@intxgo intxgo enabled auto-merge (squash) February 13, 2024 19:15
@cmacknz
Copy link
Member

cmacknz commented Feb 13, 2024

The one test failure here is unrelated:

	Received unexpected error:
        	            	failed to download https://snapshots.elastic.co/8.13.0-e2cda7bd/downloads/beats/elastic-agent/elastic-agent-8.13.0-SNAPSHOT-linux-x86_64.tar.gz: failed to write file /home/ubuntu/agent/.agent-testing/artifact/elastic-agent-8.13.0-SNAPSHOT-linux-x86_64.tar.gz: stream error: stream ID 1; INTERNAL_ERROR; received from peer

@cmacknz
Copy link
Member

cmacknz commented Feb 13, 2024

Going to force merge this one.

@cmacknz cmacknz disabled auto-merge February 13, 2024 20:16
@cmacknz cmacknz merged commit e0a2204 into elastic:main Feb 13, 2024
8 of 9 checks passed
@intxgo intxgo deleted the lesio/arm64-not-supported-on-windows branch February 13, 2024 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip enhancement New feature or request skip-changelog Team:Elastic-Agent Label for the Agent team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants