Skip to content
This repository has been archived by the owner on Oct 27, 2023. It is now read-only.

Initial implementation to create single OCi image #4

Closed
wants to merge 8 commits into from
Closed

Initial implementation to create single OCi image #4

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1ad0845
Initial implementation to create single OCi image
leo8a Oct 4, 2023
f1f8dbc
Remove oc binary from Dockerfile
leo8a Oct 5, 2023
b8a821e
Refactor nsenter cmds to use runInHostNamespace function
leo8a Oct 5, 2023
d8c3e74
Remove rpm-ostree.json from seed image
leo8a Oct 6, 2023
4989a41
Refactor runInHostNamespace function to also return rawOutput
leo8a Oct 6, 2023
e90faa9
Dump out code from github.com/coreos/rpmostree-client-go/pkg/client
leo8a Oct 6, 2023
519e5d3
Refactor rpm-ostree-client to add support for runInHostNamespace
leo8a Oct 6, 2023
e19c750
Refactor rpm-ostree cmds to use new rpm-ostree-client.go
leo8a Oct 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 54 additions & 44 deletions cmd/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/godbus/dbus"
"github.com/spf13/cobra"
"io/ioutil"
)

// authFile is the path to the registry credentials used to push the OCI image
Expand Down Expand Up @@ -92,7 +93,7 @@ func create() {
check(err)

// Create the file /var/tmp/container_list.done
err = runCMD("touch /var/tmp/container_list.done")
_, err = os.Create("/var/tmp/container_list.done")
check(err)

log.Println("List of containers saved successfully.")
Expand Down Expand Up @@ -233,66 +234,75 @@ func create() {
}

//
// Encapsulating and pushing backup OCI image
// Building and pushing OCI image
//
log.Printf("Encapsulate and push backup OCI image to %s:%s.", containerRegistry, backupTag)
log.Printf("Build and push OCI image to %s:%s.", containerRegistry, backupTag)

// Execute 'ostree container encapsulate' command for backup OCI image
ostreeEncapsulateBackupCMD := fmt.Sprintf(`nsenter --target 1 --cgroup --mount --ipc --pid sh -c 'REGISTRY_AUTH_FILE=%s ostree container encapsulate %s registry:%s:%s --repo /ostree/repo --label ostree.bootable=true'`, authFile, backupTag, containerRegistry, backupTag)
err = runCMD(ostreeEncapsulateBackupCMD)
// Get the current ostree deployment name booted and save it
bootedOSName := fmt.Sprintf(
`nsenter --target 1 --cgroup --mount --ipc --pid -- rpm-ostree status -v --json | jq -r '.deployments[] | select(.booted == true) | .osname' > /var/tmp/booted.osname`)
Copy link

Choose a reason for hiding this comment

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

BTW there is https://github.com/coreos/rpmostree-client-go/

Also unrelated nit but predictable temporary filenames are a CWE: https://cwe.mitre.org/data/definitions/377.html

Using the client-go bindings would gather info to a pipe, avoiding a temporary file at all. But short of that I'd say instead to create a temporary file on the Go side via https://pkg.go.dev/os#CreateTemp

Copy link
Owner Author

Choose a reason for hiding this comment

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

@cgwalters thanks for the comment.

did a quick test with the proposed client-go binding and I'm seeing the following issue -> <nil> exec: "rpm-ostree": executable file not found in $PATH

not sure If I might be missing something here, but rpmostree-client-go (current implementation) seems that can't run commands directly on the host namespace. Not sure if it could be used for the use case this tool is targeting.

Copy link

@cgwalters cgwalters Oct 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah I think we could add support to that project for doing the nsenter basically so it can run from a container.

Today though the way the MCO works (using this library) is it chroot()s to the host, but that has its own major tradeoffs.

Copy link
Owner Author

Choose a reason for hiding this comment

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

@cgwalters in essence, I took borrowed code from rpm-ostree-client and lifted some parts with the runInHostNamespaces function for this tool.

As suggested, I've refactored all rpm-ostree commands in e19c750 to use the new rpm-ostree-client.go and pipe gathered info by avoiding temporary files.

If the work done in 519e5d3 is interesting for the rpm-ostree-client go-binding, I'd be happy to contribute to that project by adding support to run from a container with nsenter.

err = runCMD(bootedOSName)
check(err)

//
// Encapsulating and pushing base OCI image
//
log.Printf("Encapsulate and push base OCI image to %s:%s.", containerRegistry, baseTag)

// Create base commit checksum file
ostreeBaseChecksumCMD := fmt.Sprintf(`nsenter --target 1 --cgroup --mount --ipc --pid -- rpm-ostree status -v --json | jq -r '.deployments[] | select(.booted == true).checksum' > /var/tmp/ostree.base.commit`)
err = runCMD(ostreeBaseChecksumCMD)
// Get the current ostree deployment id booted and save it
bootedID := fmt.Sprintf(
`nsenter --target 1 --cgroup --mount --ipc --pid -- rpm-ostree status -v --json | jq -r '.deployments[] | select(.booted == true) | .id' > /var/tmp/booted.id`)
err = runCMD(bootedID)
check(err)

// Read base commit from file
baseCommit, err := readLineFromFile("/var/tmp/ostree.base.commit")

// Execute 'ostree container encapsulate' command for base OCI image
ostreeEncapsulateBaseCMD := fmt.Sprintf(`nsenter --target 1 --cgroup --mount --ipc --pid sh -c 'REGISTRY_AUTH_FILE=%s ostree container encapsulate %s registry:%s:%s --repo /ostree/repo --label ostree.bootable=true'`, authFile, baseCommit, containerRegistry, baseTag)
err = runCMD(ostreeEncapsulateBaseCMD)
// Read current ostree deployment name from file
bootedOSName_, err := readLineFromFile("/var/tmp/booted.osname")
check(err)

//
// Encapsulating and pushing parent OCI image
//

// Create parent checksum file
ostreeHasParentChecksumCMD := fmt.Sprintf(`nsenter --target 1 --cgroup --mount --ipc --pid -- rpm-ostree status -v --json | jq -r '.deployments[] | select(.booted == true) | has("base-checksum")' > /var/tmp/ostree.has.parent`)
err = runCMD(ostreeHasParentChecksumCMD)
// Read current ostree deployment id from file
bootedID_, err := readLineFromFile("/var/tmp/booted.id")
check(err)

// Read hasParent commit from file
hasParent, err := readLineFromFile("/var/tmp/ostree.has.parent")
// Get booted ostree deployment sha
bootedDeployment := strings.Split(bootedID_, "-")[1]

// Check if current ostree deployment has a parent commit
if hasParent == "true" {
log.Info("OCI image has a parent commit to be encapsulated.")
// Check if the backup file for .origin doesn't exist
originFileName := fmt.Sprintf("%s/ostree-%s.origin", backupDir, bootedDeployment)
if _, err := os.Stat(originFileName); os.IsNotExist(err) {

// Create parent commit checksum file
ostreeParentChecksumCMD := fmt.Sprintf(`nsenter --target 1 --cgroup --mount --ipc --pid -- rpm-ostree status -v --json | jq -r '.deployments[] | select(.booted == true)."base-checksum"' > /var/tmp/ostree.parent.commit`)
err = runCMD(ostreeParentChecksumCMD)
// Execute 'copy' command and backup mco-currentconfig
backupOriginCMD := fmt.Sprintf(
`nsenter --target 1 --cgroup --mount --ipc --pid -- cp /ostree/deploy/%s/deploy/%s.origin %s`, bootedOSName_, bootedDeployment, originFileName)
Copy link

Choose a reason for hiding this comment

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

Also tangentially related, generating shell script like this is https://cwe.mitre.org/data/definitions/78.html in the general case.

(I've been using https://docs.rs/xshell/latest/xshell/ in some of my Rust code, it's great because Rust macros allow the ergonomics of "mini-languages" like this that correctly generate shell script while quoting input arguments)

But in Go there's nothing really else to do than passing []string to https://pkg.go.dev/os/exec#Command

Copy link

Choose a reason for hiding this comment

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

On another topic btw, it'd also probably make sense to factor out the nsenter to a helper function like runInHostNamespace(args ...string)

Copy link
Owner Author

Choose a reason for hiding this comment

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

many thanks for the comments.

I've refactored in b8a821e and now we have a runInHostNamespace function in utils.go for all the commands that need to be executed in the host.

err = runCMD(backupOriginCMD)
check(err)

// Read parent commit from file
parentCommit, err := readLineFromFile("/var/tmp/ostree.parent.commit")
log.Println("Backup of .origin created successfully.")
} else {
log.Println("Skipping .origin backup as it already exists.")
}

// Execute 'ostree container encapsulate' command for parent OCI image
log.Printf("Encapsulate and push parent OCI image to %s:%s.", containerRegistry, parentTag)
ostreeEncapsulateParentCMD := fmt.Sprintf(`nsenter --target 1 --cgroup --mount --ipc --pid sh -c 'REGISTRY_AUTH_FILE=%s ostree container encapsulate %s registry:%s:%s --repo /ostree/repo --label ostree.bootable=true'`, authFile, parentCommit, containerRegistry, parentTag)
err = runCMD(ostreeEncapsulateParentCMD)
check(err)
// Create a temporary file for the Dockerfile content
tmpfile, err := ioutil.TempFile("/var/tmp", "dockerfile-")
if err != nil {
log.Errorf("Error creating temporary file: %s", err)
}
defer os.Remove(tmpfile.Name()) // Clean up the temporary file

} else {
log.Info("Skipping encapsulate parent commit as it is not present.")
// Write the content to the temporary file
_, err = tmpfile.WriteString(containerFileContent)
if err != nil {
log.Errorf("Error writing to temporary file: %s", err)
}
tmpfile.Close() // Close the temporary file

// Build the single OCI image (note: We could include --squash-all option, as well)
leo8a marked this conversation as resolved.
Show resolved Hide resolved
containerBuildCMD := fmt.Sprintf(
`nsenter --target 1 --cgroup --mount --ipc --pid -- podman build -f %s -t %s:%s --build-context ostreerepo=/sysroot/ostree/repo %s`,
tmpfile.Name(), containerRegistry, backupTag, backupDir)
err = runCMD(containerBuildCMD)
check(err)

// Push the created OCI image to user's repository
containerPushCMD := fmt.Sprintf(
`nsenter --target 1 --cgroup --mount --ipc --pid -- podman push --authfile %s %s:%s`,
authFile, containerRegistry, backupTag)
err = runCMD(containerPushCMD)
check(err)

log.Printf("OCI image created successfully!")
}
17 changes: 9 additions & 8 deletions cmd/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,8 @@ import (
)

const (
// Default backup tag
backupTag = "backup"
// Default base tag
baseTag = "base"
// Default parent tag
parentTag = "parent"
// Default ostreeAuthFile location
ostreeAuthFile = "/run/ostree/auth.json"
// Default OCI image tag
backupTag = "oneimage"
// Pull secret. Written by the machine-config-operator
imageRegistryAuthFile = "/var/lib/kubelet/config.json"
// sourceDir is the directory where the datadir is backed up
Expand All @@ -43,6 +37,13 @@ const (
kubeconfigFile = "/etc/kubernetes/static-pod-resources/kube-apiserver-certs/secrets/node-kubeconfigs/lb-ext.kubeconfig"
)

// containerFileContent is the Dockerfile content for the IBU seed image
const containerFileContent = `
FROM scratch
COPY . /
COPY --from=ostreerepo . /ostree/repo
Copy link

@cgwalters cgwalters Oct 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ah I see. Humm...so one definite downside of this is will actually defeat the content-addressed layers in our bootable base image, effectively reducing everything to one large layer. Not fatal...but also not ideal either.

So today what would probably work is to do this:

FROM <rhel coreos>
COPY etc.tar.gz /tmp/etc.tar.gz
COPY var.tar.gz /tmp/var.tar.gz

(Or make it not a tarball and just use /tmp/etc)

Then this image could on the node be:

rpm-ostree rebase $oneimage

The rebase command should warn-and-drop all the content in /tmp right now.

Then, we'd need to pull it again via podman today (yes this is unfortunate but we're not yet at unified storage) and extract the files we need from /tmp/etc and /tmp/var.

And then the last step would be one more rpm-ostree rebase $realbase to drop the extra layers and go back to the real base image.

Copy link

Choose a reason for hiding this comment

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

Now...a while ago I did coreos/rpm-ostree#3961 which is solving a very similar problem and runs on every boot from an old (pre 4.12) bootimage where rpm-ostree is too old to natively understand containers. There we actually do the pull from podman and do what you're aiming to do here with ostree pull-local from the privileged container.

The unfortunate part is this flow doesn't yet set things up with a proper container origin so we end up taking another reboot unfortunately...I think we could probably fix that. But it's just...so messy and hard to have code that runs both inside and outside of a container image right now.

Copy link
Owner Author

Choose a reason for hiding this comment

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

bringin' also @donpenney and @javipolo to this thread, in case they want / have something to comment on this proposed flow

Copy link

Choose a reason for hiding this comment

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

BTW did you guys see my thoughts over at containers/bootc#22 ?

IMO basically what we should do with most of the current Ignition stuff is move it into just a derived image. I see "attached configmaps" replacing the bits that need to be dynamic or even per machine.

However you are effectively making the case here that we may want some sort of first-class support for something a bit like a "release image" at the OS level that actually is just a "rollup" of a reference to one or more container images and configmaps potentially.

`

func check(err error) {
if err != nil {
log.Errorf("An error occurred: %s", err.Error())
Expand Down