smee icon indicating copy to clipboard operation
smee copied to clipboard

Remove references to packet in auto.ipxe for OSIE installer:

Open jacobweinstock opened this issue 2 years ago • 2 comments

Description

With the recent addition of the CLI flag -extra-kernel-args all of the packet (equinix metal) specific kernel args in the auto.ipxe of the OSIE installer can now be specified at runtime instead of compiled in.

Why is this needed

Fixes: #

How Has This Been Tested?

How are existing users impacted? What migration steps/scripts do we need?

Checklist:

I have:

  • [ ] updated the documentation and/or roadmap (if required)
  • [ ] added unit or e2e tests
  • [ ] provided instructions on how to upgrade

jacobweinstock avatar May 07 '22 02:05 jacobweinstock

Codecov Report

Merging #267 (d043632) into main (590fe8a) will decrease coverage by 0%. The diff coverage is n/a.

@@         Coverage Diff         @@
##           main   #267   +/-   ##
===================================
- Coverage    24%    24%   -1%     
===================================
  Files        28     28           
  Lines      1660   1648   -12     
===================================
- Hits        402    397    -5     
+ Misses     1222   1217    -5     
+ Partials     36     34    -2     
Impacted Files Coverage Δ
conf/config.go 33% <ø> (ø)
installers/osie/main.go 65% <ø> (+2%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov[bot] avatar May 07 '22 02:05 codecov[bot]

Sorry for the late response. But I think @nshalman and @mmlb need to look at this as well.

The logic for some of the removed arguments won't be able to be added with a static extra-kernel-args. Specifically the Hollow variables which should only be included on a deprovision.

With this change, it looks like we'll need to include in deployments the following extra arguments to match the previous arguments.

alpine_repo=${base-url}/repo-${arch}/main
modloop=${base-url}/modloop-${parch}
packet_action=${action}
packet_state=${state}
osie_vendors_url=$OSIE_VENDOR_SERVICES_URL
packet_bootdev_mac=${bootdevmac}

As well as conditionally

hollow_client_id=$HOLLOW_CLIENT_ID
hollow_client_request_secret=$HOLLOW_CLIENT_REQUEST_SECRET
packet_base_url=${base-url}

And packet_base_url=$MIRROR_BASE_URL/workflow when running in workflow mode.

I like the idea of this, but with the tinkerbell/osie being archived. Perhaps the osie boots installer package should remain mostly unchanged, and a new default installer with better configurability should be created instead? Just a thought. I'd like to see what @nshalman and @mmlb have to say.

Hey @mikemrm , thanks for the review. Providing optional auto.ipxe logic sounds like something we need to figure out and a great example of use-cases we need to solve in a generic way instead of keeping hard coded Equinix only logic like this.

Are there certain packet/equinix references you think should be good to remove now? and any specific references that would impact the Equinix Metal specific deployment of Boots?

jacobweinstock avatar May 11 '22 02:05 jacobweinstock

Sorry for the late response. But I think @nshalman and @mmlb need to look at this as well. The logic for some of the removed arguments won't be able to be added with a static extra-kernel-args. Specifically the Hollow variables which should only be included on a deprovision. With this change, it looks like we'll need to include in deployments the following extra arguments to match the previous arguments.

alpine_repo=${base-url}/repo-${arch}/main
modloop=${base-url}/modloop-${parch}
packet_action=${action}
packet_state=${state}
osie_vendors_url=$OSIE_VENDOR_SERVICES_URL
packet_bootdev_mac=${bootdevmac}

As well as conditionally

hollow_client_id=$HOLLOW_CLIENT_ID
hollow_client_request_secret=$HOLLOW_CLIENT_REQUEST_SECRET
packet_base_url=${base-url}

And packet_base_url=$MIRROR_BASE_URL/workflow when running in workflow mode. I like the idea of this, but with the tinkerbell/osie being archived. Perhaps the osie boots installer package should remain mostly unchanged, and a new default installer with better configurability should be created instead? Just a thought. I'd like to see what @nshalman and @mmlb have to say.

Hey @mikemrm , thanks for the review. Providing optional auto.ipxe logic sounds like something we need to figure out and a great example of use-cases we need to solve in a generic way instead of keeping hard coded Equinix only logic like this.

Are there certain packet/equinix references you think should be good to remove now? and any specific references that would impact the Equinix Metal specific deployment of Boots?

Hey @mikemrm, in response to Perhaps the osie boots installer package should remain mostly unchanged, and a new default installer with better configurability should be created instead?

That definitely is an option. I think it just pushes the buck down the road though. Packet/Equinix specific references need to be removed regardless. If we go down the path of creating a new default installer I'd request that you and @mmlb, @nshalman, @ScottGarman provide a deprecation plan for this current osie installer and share it. That way we can all help move it along.

jacobweinstock avatar Oct 03 '22 15:10 jacobweinstock

I think we at Equinix Metal need to get this unblocked. It might take us longer than ideal to fully pivot off these bits. The two main options in my mind are:

  • Metal forks the repo to buy us time to do our migration while unblocking this work
  • The Tinkerbell project allows us to maintain our production branch within the project while we unblock this work and then get ourselves back onto the main branch.

~~@jacobweinstock Do you have any opinions on fork vs branch within the project?~~

I've been informed that we'll be forking. We'll be out of the way shortly...

nshalman avatar Oct 06 '22 12:10 nshalman

I think we at Equinix Metal need to get this unblocked. It might take us longer than ideal to fully pivot off these bits. The two main options in my mind are:

  • Metal forks the repo to buy us time to do our migration while unblocking this work
  • The Tinkerbell project allows us to maintain our production branch within the project while we unblock this work and then get ourselves back onto the main branch.

~@jacobweinstock Do you have any opinions on fork vs branch within the project?~

I've been informed that we'll be forking. We'll be out of the way shortly...

This is definitely not what I wanted to see, very unfortunate for both sides.

jacobweinstock avatar Jan 06 '23 18:01 jacobweinstock