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

Added an example on how to deploy all the edge components #331

Closed
wants to merge 0 commits into from

Conversation

e-minguez
Copy link
Contributor

No description provided.

@e-minguez e-minguez marked this pull request as draft March 22, 2024 15:45
# Same for metal3
cp metal3.sh /opt/edge/bin/
chmod a+x /opt/edge/bin/metal3.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#200 will simplify this a lot

Comment on lines 7 to 45
kubernetes:
helm:
charts:
- createNamespace: true
installationNamespace: kube-system
name: cert-manager
repositoryName: jetstack
targetNamespace: cert-manager
valuesFile: certmanager.yaml
version: 1.14.2
- createNamespace: true
installationNamespace: kube-system
name: longhorn
repositoryName: longhorn
targetNamespace: longhorn-system
version: 1.6.0
- createNamespace: true
installationNamespace: kube-system
name: rancher
repositoryName: rancher-stable
targetNamespace: cattle-system
valuesFile: rancher.yaml
version: 2.8.2
- createNamespace: true
installationNamespace: kube-system
name: metal3
repositoryName: suse-edge
targetNamespace: metal3-system
valuesFile: metal3.yaml
version: 0.6.3
repositories:
- name: jetstack
url: https://charts.jetstack.io
- name: longhorn
url: https://charts.longhorn.io
- name: rancher-stable
url: https://releases.rancher.com/server-charts/stable
- name: suse-edge
url: https://suse-edge.github.io/charts
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#306 will simplify this a lot

@@ -0,0 +1,4 @@
hostname: rancher-192.168.122.11.sslip.io
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This requires the nginx ingress configuration tweaks

@@ -0,0 +1,25 @@
apiVersion: apps/v1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just an example of a workload being deployed

@@ -0,0 +1,128 @@
#!/bin/bash
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to find a better way to do this 😅

@@ -0,0 +1,19 @@
# Based on https://gist.github.com/agracey/6966338d34c7277f5b5ec6f4c1431b1b
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using your example here @agracey is this right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jdob
Copy link
Contributor

jdob commented Mar 22, 2024

One quick request: can you add an entry under the Advanced section of the README.md in the examples directory? There is a lot going on in this example (which is awesome) but mere mortals aren't going to have a clue what all is in it.

@e-minguez
Copy link
Contributor Author

One quick request: can you add an entry under the Advanced section of the README.md in the examples directory? There is a lot going on in this example (which is awesome) but mere mortals aren't going to have a clue what all is in it.

Yes but no :D

I would like to first discuss the approach before writing the docs just in case it changes a lot.

kind: Namespace
metadata:
labels:
pod-security.kubernetes.io/enforce: privileged
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@atanasdinov atanasdinov left a comment

Choose a reason for hiding this comment

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

Looks great!

Some minor comments but we definitely need to add docs for this before merging as @jdob pointed out.

examples/edge-stack-iso/kubernetes/config/agent.yaml Outdated Show resolved Hide resolved
examples/edge-stack-iso/kubernetes/config/server.yaml Outdated Show resolved Hide resolved
examples/edge-stack-iso/custom/scripts/99_edge-setup.sh Outdated Show resolved Hide resolved
Comment on lines 15 to 22
# Get or create the lock to run all those steps just in a single node
# As the first node is created WAY before the others, this should be enough
# TODO: Investigate if leases is better
if [ $(${KUBECTL} get cm -n ${RANCHERLOCKNAMESPACE} ${RANCHERLOCKCMNAME} -o name | wc -l) -lt 1 ]; then
${KUBECTL} create configmap ${RANCHERLOCKCMNAME} -n ${RANCHERLOCKNAMESPACE} --from-literal foo=bar
else
exit 0
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about writing a file to the filesystem instead? We'll end up here for all nodes just because there's a cleanup of the config map in the end of the script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The point of this 'lock' is because this script will be executed on all the hosts, so it is a poor's man 'distributed/shared lock' :D If a file is being written by one of the hosts, the others won't be able to see it (unless I'm missing something)

Copy link
Contributor

Choose a reason for hiding this comment

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

This is precisely the case. Isn't Rancher supposed to be bootstrapped just once?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry but I feel I'm missing something. The script will be executed on all the hosts being provisioned as there is no way to make a distinction right now on EIB. If there is no lock it can happen that all the hosts are waiting for rancher to be up (next line) and then they try to create the password, etc. at the same time and then the script may fail on those that are not the first one.
If I create a file locally, it will be created on each host, leading to the previous scenario as well as there is no 'sync' between them in that case.

examples/edge-stack-iso/custom/files/metal3.sh Outdated Show resolved Hide resolved
examples/edge-stack-iso/custom/files/metal3.sh Outdated Show resolved Hide resolved
examples/edge-stack-iso/custom/files/metal3.sh Outdated Show resolved Hide resolved
esac

# Clusterctl bin
# Maybe just use the binary from hauler if available
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest dropping this comment. If we want to sideload the binary, we should instead just put it under /custom/files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As hauler allows you to pack binaries (https://rancherfederal.github.io/hauler-docs/docs/guides-references/hauler-content/files) as well I think it would be cleaner if we can add a flow to EIB to save binaries to hauler somehow.

hauler store add file https://github.com/kubernetes-sigs/cluster-api/releases/download/v${METAL3_CLUSTERCTLVERSION}/clusterctl-linux-${GOARCH}

I can put the binary directly I agree but I don't want to do save it directly into git and if I don't save it the example will not be 'complete'.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Keeping the download is completely fine for now. On a separate note, Hauler is intentionally not being used for files right now and this might very well be the case in the future as well but it's something we'll discuss when we have the time.

@e-minguez
Copy link
Contributor Author

Looks great!

Some minor comments but we definitely need to add docs for this before merging as @jdob pointed out.

I'll take a look at the comments, thanks.

WRT docs I agree but I want to get all the inputs first then clean all the reviews/suggestions before just in case it changes too much.

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.

4 participants