Skip to content

MacOS support - #1679

Merged
KevinFairise2 merged 7 commits into
mainfrom
kfairise/macos-support
Aug 28, 2025
Merged

MacOS support#1679
KevinFairise2 merged 7 commits into
mainfrom
kfairise/macos-support

Conversation

@KevinFairise2

@KevinFairise2 KevinFairise2 commented Aug 26, 2025

Copy link
Copy Markdown
Member

What does this PR do?

Add support for MacOS instances. This PR allows to:

  • Run a macOS instances (arm64 or x64)
  • Install the agent on it using the install-script
  • Allow to target the agent of a given pipeline

The goal is to create smoke tests on datadog-agent repository to make sure the macos agent can be installed.

Which scenarios this will impact?

Motivation

Additional Notes

Note that dedicated host must be running for 24 hours before they can be deleted. As a consequences we cannot delete the dedicated host with Pulumi. Pulumi will leave it not destroyed and test-infra-cleaner should be responisble for cleaning dedicated host when possible.

@KevinFairise2
KevinFairise2 force-pushed the kfairise/macos-support branch from 2f80901 to 8225861 Compare August 27, 2025 12:49
@KevinFairise2 KevinFairise2 changed the title wip MacOS support Aug 27, 2025
@KevinFairise2
KevinFairise2 force-pushed the kfairise/macos-support branch from 8225861 to 4458411 Compare August 27, 2025 14:10
@KevinFairise2
KevinFairise2 marked this pull request as ready for review August 27, 2025 14:48
@KevinFairise2
KevinFairise2 requested a review from a team as a code owner August 27, 2025 14:48
@KevinFairise2
KevinFairise2 force-pushed the kfairise/macos-support branch from 71a7ab9 to 2ef6f4d Compare August 27, 2025 14:56
Comment thread README.md Outdated
Comment thread components/datadog/agent/host_macos.go Outdated
Comment thread components/datadog/agent/package.go Outdated
Comment on lines +47 to +48
case tifos.MacosOS:
wantedExt = ".dmg"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT: Could you add the fallthrough statement with tifos.Unknown? Like you did above

var (
MacOSDefault = MacOSSonoma
MacOSSonoma = NewDescriptorWithArch(MacosOS, "sonoma", ARM64Arch)
MacOSSonoma = NewDescriptor(MacosOS, "sonoma")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we define other versions such as ventura etc. ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Or is it an aws limitation?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think other version are supported, at least sequoia exists. But we can probably add them later if they are needed

Comment thread components/os/const.go
archStr = strings.ToLower(archStr)
switch archStr {
case "x86_64", "amd64", "": // Default architecture is AMD64
case "x86_64", "amd64", "", "x86_64_mac": // Default architecture is AMD64

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question: Why is the default architecture amd64? Since we have arm builds now maybe we want to change that?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Since this function set the default for all the instances and not only Macos I think it makes sense to keep AMD64 as the default. Changing that would probably change the default architecture for all the existing tests which would be quite a big change


// Optional
AvailabilityZone string // If not specified, will use first available zone
HostRecovery string // "on" or "off", defaults to "off"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT: Should we use a boolean for that?

Comment thread resources/aws/ec2/dedicated_host.go Outdated
Comment thread resources/aws/environmentDefaults.go Outdated
Comment thread scenarios/aws/ec2/vm_run.go Outdated
userData string
instanceType string
instanceProfile string
tenancy string

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT: Should we declare WithTenancy?

Co-authored-by: Célian Raimbault <161456554+CelianR@users.noreply.github.com>
Comment thread components/datadog/agent/host_macos.go Outdated
Comment thread components/datadog/agent/host_macos.go
Comment thread components/datadog/agent/host_os.go
Comment thread components/datadog/agent/package.go Outdated
@KevinFairise2
KevinFairise2 force-pushed the kfairise/macos-support branch from b6e1995 to 5ad4dcf Compare August 28, 2025 09:48
@KevinFairise2
KevinFairise2 merged commit ce15fd8 into main Aug 28, 2025
8 checks passed
@KevinFairise2
KevinFairise2 deleted the kfairise/macos-support branch August 28, 2025 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants