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

Adding Mac AMI Type support to launch mac1.metal instances by ec2-plugin #585

Merged
merged 30 commits into from
Apr 13, 2021
Merged

Adding Mac AMI Type support to launch mac1.metal instances by ec2-plugin #585

merged 30 commits into from
Apr 13, 2021

Conversation

ykhandelwal913
Copy link
Contributor

@ykhandelwal913 ykhandelwal913 commented Mar 22, 2021

Adding following feature as part of the PR

  • Adding support for macType in AmiType.
  • Written separate classes for macSupport. In case in future, something specific needed for macOS. Currently it is like UnixLauncher but having java installation and url changes. Using aws corretto java.
  • Written Testcases to validate new Type.
    This PR has a dependency on Add Support for Host Tenancy Option #492 as mac1.metal instances need HostTenancy.

@ykhandelwal913 ykhandelwal913 changed the title Adding MacType support to launch mac1.metal instances by ec2-plugin Adding Mac AMI Type support to launch mac1.metal instances by ec2-plugin Mar 23, 2021
@ykhandelwal913
Copy link
Contributor Author

@res0nance can you label it as enhancement.

@res0nance
Copy link
Contributor

I do not understand this change, the new launcher simply looks like a copy of UnixLauncher so why can't we use that instead?

@ykhandelwal913
Copy link
Contributor Author

@res0nance installation of java would be different in case of OSX.

@ykhandelwal913
Copy link
Contributor Author

I could use System.getProperty("os.name") and added check in unix launcher to solve it but kept a new class and other files separate for mac, in case we want to add more functionality related to mac. let me know your thoughts, i can add system property changes and have another PR.

@ykhandelwal913
Copy link
Contributor Author

@res0nance In future if there is a drift having different path(linux and mac) is helpful

@res0nance
Copy link
Contributor

I think the change is ok but i lack the infrastructure to test this properly. Have you tested these changes?

@ykhandelwal913
Copy link
Contributor Author

yes i have validated this changes.

@res0nance res0nance added the enhancement Feature additions or enhancements label Apr 3, 2021
@taneishamitchell
Copy link

@res0nance who's responsible for version releases?
I am so looking forward to these tenancy changes to be available

@res0nance
Copy link
Contributor

@res0nance who's responsible for version releases?
I am so looking forward to these tenancy changes to be available

Technically any maintainer can release it

@res0nance res0nance merged commit 80d6bb1 into jenkinsci:master Apr 13, 2021
@taneishamitchell
Copy link

taneishamitchell commented Apr 13, 2021

Technically any maintainer can release it

Is there any hopes of that happening soon after the tests on the master branch is passing? A girl is desperate 😞

@a-pikachu
Copy link

Is there any documentation on how to config this? Especially the dedicated host part. Do we need to first reserved a host or the plugin will do it for us? Thanks

@ykhandelwal913
Copy link
Contributor Author

This plugin won't reserve the host. You will have to reserve the host. This plugin can help you to launch the mac1.metal instance on the dedicated host.

@a-pikachu
Copy link

in https://docs.aws.amazon.com/powershell/latest/reference/items/New-EC2Instance.html

I see this
New-EC2Instance -ImageId ami-1a2b3c4d -InstanceType m4.large -KeyName my-key-pair -SecurityGroupId sg-1a2b3c4d -AvailabilityZone us-west-1a -Tenancy host -HostID h-1a2b3c4d5e6f1a2b3
Launches an instance into the specified dedicated host environment.

But with option in the plugin only have Tenancy choice, where to pass the host ID?

image

@ykhandelwal913
Copy link
Contributor Author

You have to turn on the Instance auto-placement – Enable auto-placement to allow the host to accept untargeted instance launches that match its host configuration. By default, it will not be enabled.

@a-pikachu
Copy link

@ykhandelwal913 I am new to AWS so can you point me to where to turn on the instance auto-placement? Is it in the AWS panel? Thanks

@ykhandelwal913
Copy link
Contributor Author

@a-pikachu sorry for the delay in reply. Either when you are reserving the instance you can pass the parameter like aws ec2 allocate-hosts --instance-family Mac1 --availability-zone us-east-1a --auto-placement on --quantity x or change/create it from aws panel->EC2-> DedicatedHosts

@a-pikachu
Copy link

@ykhandelwal913 thank you, I have it resolved now by enabling auto-placement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature additions or enhancements
Projects
None yet
5 participants