Progress on Proposal 0016, Default Workflows #179

Belchy06 · 2021-07-22T03:36:30Z

Description

This PR adds the ability for Boots to push hardware from a currently unknown mac address.

Currently the hardware template is a hardcoded, minimalistic template from the hello-world tutorial. Proper hardware discovery, or input from a default hardware file will be needed.

Why is this needed

This feature is required for the project I am working on but I am sure there are many other use cases I'm unaware of. Also, #178

This PR requires additional functionality from the TInk-CLI which can be seen: tinkerbell/tink#516

How Has This Been Tested?

This has been tested in the VM's based on the Local Setup with Vagrant tutorial.

If the env var ENABLE_DEFAULT_WORKFLOWS is not set, Boots will follow it's original code path meaning that mass destruction can be prevented

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

…ady exist Signed-off-by: William Belcher <[email protected]> Signed-off-by: Belchy06 <[email protected]>

…ardware from unknown mac Signed-off-by: William Belcher <[email protected]> Signed-off-by: Belchy06 <[email protected]>

Signed-off-by: Belchy06 <[email protected]>

tstromberg

Some minor changes, will defer to @jacobweinstock for a more detailed review.

tstromberg · 2021-08-27T16:55:53Z

ipxe/ipxe/build.sh

@@ -3,7 +3,7 @@
 set -eu

 # Deps on ubuntu
-# apt-get -y --no-install-recommends install build-essential gcc-aarch64-linux-gnu git liblzma-dev
+sudo apt-get -y --no-install-recommends install build-essential gcc-aarch64-linux-gnu git liblzma-dev


The changes to build.sh seem unrelated - can you move them to another PR?

tstromberg · 2021-08-27T16:57:51Z

job/fetch.go

+	if err != nil {
+		return nil, err
+	}
+	return v.(packet.Discovery), nil


This should catch a type assertion error instead of panic'ing:

d, ok := v.(packet.Discovery) if !ok { return nil, fmt.Errorf("unable to cast %+v to packet.Discovery") } return d, nil

tstromberg · 2021-08-27T16:58:40Z

job/job.go

@@ -90,6 +112,25 @@ func CreateFromDHCP(mac net.HardwareAddr, giaddr net.IP, circuitID string) (Job,
 	return j, nil
 }

+// CreateWorkflowFromDHCP creates a workflow and
+func CreateHWFromDHCP(mac net.HardwareAddr, giaddr net.IP, circuitID string) (Job, error) {


Should this be called CreateWorkflowFromDHCP?

tstromberg · 2021-08-27T16:59:47Z

packet/endpoints.go

+
+	b, err := json.Marshal(&req)
+	if err != nil {
+		return nil, errors.Wrap(err, "unmarshalling api discovery")


the error should be marshaling. errors.Wrap(err, "marshal") is honestly good enough.

jacobweinstock · 2021-08-27T18:48:49Z

@Belchy06 thanks so much for this work! I want to make sure I understand fully what you're looking to do. Do I have this correct?

hardware discovery
- a machine requesting a PXE boot without a corresponding Hardware record in Tink Server will cause Boots to push a new minimal Hardware record to Tink Server.
default workflow
- a machine requesting a PXE boot without a corresponding Workflow in Tink Server will cause Boots to create a new Workflow in Tink Server using an existing Template named "default" and the MAC address in the DHCP request.

Assuming these 2 are accurate statements, the following might need to be discussed further.

hardware discovery

I think we might need to implement some kind of IP Pool feature from which an IP can be used in the creation of a Tink Server Hardware record.

default workflow

This could potentially be functionality that lives entirely in Tink Server

Belchy06 · 2021-08-27T23:49:26Z

@jacobweinstock Your two points were exactly my train of thought.

I implemented the IP assignment functionality on a separate branch here. In this case the user just specifies the starting IP and a lease range and Boots takes care of the rest.

This solution feels quite 'hacky' and feel like quality code, hence why it's not in this PR.

Keen to discuss your ideas further!

djpbessems · 2021-09-10T09:48:07Z

Very interested in this; this could allow my Tinkerbell Appliance to automatically build an inventory from which endusers will be able to select and assign images. Would be super :)

tstromberg · 2021-09-21T15:23:01Z

@Belchy06 - would you have time to resolve the PR conflicts & minor issues I raised?

tstromberg · 2021-10-05T15:34:28Z

@Belchy06 - Do you need any additional help or advice to push this PR forward? We'd be happy to discuss it here, on the Tinkerbell Slack, or in the community meeting next week: https://tinkerbell.org/community/

Belchy06 · 2021-10-07T01:00:26Z

@tstromberg Sorry Thomas.

I'm currently in the middle of my university semester and juggling that with work as led to this falling to the back burner.

I've got around 6-7 weeks before I'll be able to commit some of my time again to working on this, and I am happy to fix the issues and get this merged when I've got the time again.

If that's too long just let me know.

jacobweinstock · 2021-10-19T15:27:33Z

Hey @Belchy06, thank you so much for working on this! No worries at all for the delay. I'm going to close this for now, but please do re-open this when you are back with time. Best of luck and well wishes on your studies!

jmpolom · 2021-10-24T18:26:03Z

This is a super interesting concept that I’d like to suggest we examine generalizing further. It would be extremely nice if boots could support functioning without the tink workflow engine for cases where it will be used to provision a relatively small set of systems and an ultra lightweight deployment is desirable. I will get around to opening a separate issue to detail what I think would be a useful feature set here, but for now I’d like to convey that this is stellar initial work.

nicklasfrahm · 2022-02-15T16:54:36Z

I will throw in a design doc once I am done with some work on the integration with Kubernetes.

jmpolom · 2022-02-15T19:47:05Z

@nicklasfrahm is there a way to provide input and feedback on this design document? I am extremely interested in participating in the design of this feature.

nicklasfrahm · 2022-02-15T19:48:40Z

Absolutely, I will poke you once I start the PR. We could hop on call or something.

jmpolom · 2022-02-15T20:11:32Z

@nicklasfrahm yes please reach out. When do you envision starting this process? I could accommodate a call on pretty short notice but would prefer to plan something out a bit if possible.

nicklasfrahm · 2022-02-15T20:13:13Z

Are you on the CNCF slack? Throw me a DM there and we'll make it work!

jmpolom · 2022-02-15T20:17:07Z

I am not. I believe that is invite only? Can you send me an invite?

Belchy06 force-pushed the default-workflow branch from 4747c15 to 26be052 Compare July 22, 2021 03:38

Belchy06 added 4 commits July 24, 2021 13:32

Add primitive creation of hardware if hardware template does not alre…

4793019

…ady exist Signed-off-by: William Belcher <[email protected]> Signed-off-by: Belchy06 <[email protected]>

Update boots to check for ENABLE_DEFAULT_WORKFLOWS before pushing h…

1df140f

…ardware from unknown mac Signed-off-by: William Belcher <[email protected]> Signed-off-by: Belchy06 <[email protected]>

add endpoints and functionality for boots to create workflows

d4d02d6

Signed-off-by: Belchy06 <[email protected]>

Remove references to files from local development

8f50dc2

Signed-off-by: Belchy06 <[email protected]>

Belchy06 force-pushed the default-workflow branch from f29eace to 8f50dc2 Compare July 24, 2021 03:33

Merge branch 'master' into default-workflow

b27881b

Belchy06 marked this pull request as ready for review July 29, 2021 05:52

jacobweinstock linked an issue Aug 24, 2021 that may be closed by this pull request

Proposal 0016, Default Workflows #178

Open

tstromberg suggested changes Aug 27, 2021

View reviewed changes

jacobweinstock closed this Oct 19, 2021

jmpolom mentioned this pull request Jan 4, 2022

Proposal 0016, Default Workflows #178

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Progress on Proposal 0016, Default Workflows #179

Progress on Proposal 0016, Default Workflows #179

Belchy06 commented Jul 22, 2021 •

edited

Loading

tstromberg left a comment

tstromberg Aug 27, 2021

tstromberg Aug 27, 2021

tstromberg Aug 27, 2021

tstromberg Aug 27, 2021

jacobweinstock commented Aug 27, 2021 •

edited

Loading

Belchy06 commented Aug 27, 2021

djpbessems commented Sep 10, 2021

tstromberg commented Sep 21, 2021

tstromberg commented Oct 5, 2021

Belchy06 commented Oct 7, 2021

jacobweinstock commented Oct 19, 2021

jmpolom commented Oct 24, 2021

nicklasfrahm commented Feb 15, 2022

jmpolom commented Feb 15, 2022

nicklasfrahm commented Feb 15, 2022

jmpolom commented Feb 15, 2022

nicklasfrahm commented Feb 15, 2022

jmpolom commented Feb 15, 2022 •

edited

Loading

Progress on Proposal 0016, Default Workflows #179

Progress on Proposal 0016, Default Workflows #179

Conversation

Belchy06 commented Jul 22, 2021 • edited Loading

Description

Why is this needed

How Has This Been Tested?

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

Checklist:

tstromberg left a comment

Choose a reason for hiding this comment

tstromberg Aug 27, 2021

Choose a reason for hiding this comment

tstromberg Aug 27, 2021

Choose a reason for hiding this comment

tstromberg Aug 27, 2021

Choose a reason for hiding this comment

tstromberg Aug 27, 2021

Choose a reason for hiding this comment

jacobweinstock commented Aug 27, 2021 • edited Loading

hardware discovery

default workflow

Belchy06 commented Aug 27, 2021

djpbessems commented Sep 10, 2021

tstromberg commented Sep 21, 2021

tstromberg commented Oct 5, 2021

Belchy06 commented Oct 7, 2021

jacobweinstock commented Oct 19, 2021

jmpolom commented Oct 24, 2021

nicklasfrahm commented Feb 15, 2022

jmpolom commented Feb 15, 2022

nicklasfrahm commented Feb 15, 2022

jmpolom commented Feb 15, 2022

nicklasfrahm commented Feb 15, 2022

jmpolom commented Feb 15, 2022 • edited Loading

Belchy06 commented Jul 22, 2021 •

edited

Loading

jacobweinstock commented Aug 27, 2021 •

edited

Loading

jmpolom commented Feb 15, 2022 •

edited

Loading