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

Add teal flavor #1259

Merged
merged 17 commits into from
May 2, 2022
Merged

Add teal flavor #1259

merged 17 commits into from
May 2, 2022

Conversation

mudler
Copy link
Contributor

@mudler mudler commented Apr 26, 2022

Add teal flavor to our pipelines.

Part of #1238

Signed-off-by: Ettore Di Giacinto [email protected]

Add teal flavor to our pipelines.

Part of #1238

Signed-off-by: Ettore Di Giacinto <[email protected]>
Signed-off-by: Ettore Di Giacinto <[email protected]>
Teal don't have any online repos where we can install packages from

Signed-off-by: Ettore Di Giacinto <[email protected]>
Signed-off-by: Ettore Di Giacinto <[email protected]>
Signed-off-by: Ettore Di Giacinto <[email protected]>
Signed-off-by: Ettore Di Giacinto <[email protected]>
Signed-off-by: Ettore Di Giacinto <[email protected]>
Signed-off-by: Ettore Di Giacinto <[email protected]>
Signed-off-by: Ettore Di Giacinto <[email protected]>
Signed-off-by: Ettore Di Giacinto <[email protected]>
@mudler mudler marked this pull request as ready for review April 28, 2022 17:12
Signed-off-by: Ettore Di Giacinto <[email protected]>
@mudler mudler removed the arm64 label Apr 29, 2022
mudler and others added 3 commits April 29, 2022 15:09
Signed-off-by: Ettore Di Giacinto <[email protected]>
Signed-off-by: Ettore Di Giacinto <[email protected]>
@mudler
Copy link
Contributor Author

mudler commented Apr 29, 2022

Most of this is just adding new pipelines, so it comes with a lot of additions.

On a package-tree related changes it does the following:

  • Adds a teal flavor - the main difference is that all packages are already there, so no need to zypper in anything (or almost)
  • it introduces a tool package/image which is used aside for ops. It basically matches the hardcoded image we had previously in the grub package to extract the relevant efi images, but now it is exposed directly to the values file
  • the tool image is now used also for other things like as base of golang builds etc, making things well separated, and if in case of needs to build directly against the base image, it's still possible
  • Introduce a bit more templating and reduce dups across packages

@mudler
Copy link
Contributor Author

mudler commented Apr 29, 2022

I'd go ahead with this and check out manually the artifacts to see if there are any issues and will double check with the ones we currently have for green, I bet there will be some small adjustments to do

@@ -3,6 +3,16 @@ requires:
category: "distro"
version: ">=0"

excludes:
- ^/boot/grub2
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 think those two previously just slipped in, no real need to add them as part of the kernel package

upx bin/rancherd && \
cp bin/rancherd /usr/bin/
./scripts/build
{{ if .Values.upx }}
Copy link
Contributor Author

@mudler mudler Apr 29, 2022

Choose a reason for hiding this comment

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

as a low hanging fruit, I think upx should be moved as a global value and make all golang binary optionally spit upx'ed bins. In this way we can see if we can reduce binary sizes. I remind #356 hitting because of upx, but we never experimented with lower compression ratios.

if err != nil {
panic(err)
fmt.Println("No packages found")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we don't find any package (because the repo is new) we previously panic'ed. I think we did this as a safeguard mechanism, but in any case we do repo snapshotting now, so we shouldn't care much.

@@ -7,16 +7,9 @@ package_dir: /{{.Values.name}}

env:
- PATH=$PATH:/usr/local/go/bin
- CGO_ENABLED=0
Copy link
Contributor Author

@mudler mudler Apr 29, 2022

Choose a reason for hiding this comment

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

somehow those slipped in 🤷‍♂️ I added those back as teal was failing trying to run ld, so I noticed that we were building with cgo on. I'm pretty sure we didn't do that on purpose, we used to disable this only on fips builds

{{if eq .Values.codename "teal" }}
unpack: true
includes:
- ^/boot|^/boot/.*
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 real difference in teal is that we extract all the kernels from the image, so no zypper in or build happening

@@ -1,4 +1,6 @@
{{ define "distro_install" }}
## All packages for teal are in the teal image already.
{{ if ne .Values.codename "teal" }}
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 gating accidental calls to zypper in.. if we are in teal. supposedly, we don't add any repo to it

@mudler mudler enabled auto-merge (squash) April 30, 2022 22:03
Copy link
Contributor

@mjura mjura left a comment

Choose a reason for hiding this comment

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

LGTM

@mudler mudler merged commit 5bf6a91 into master May 2, 2022
@mudler mudler deleted the teal branch May 2, 2022 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants