Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions pkg/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,10 @@ func New(

// Only pull the osImageURL from OSTree when we are on RHCOS or FCOS
if os.IsCoreOSVariant() {
err := nodeUpdaterClient.Initialize()
if err != nil {
return nil, fmt.Errorf("error initializing rpm-ostree: %v", err)
}
osImageURL, osVersion, err = nodeUpdaterClient.GetBootedOSImageURL()
if err != nil {
return nil, fmt.Errorf("error reading osImageURL from rpm-ostree: %v", err)
Expand Down
65 changes: 58 additions & 7 deletions pkg/daemon/rpm-ostree.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const (
// https://github.com/projectatomic/rpm-ostree/blob/bce966a9812df141d38e3290f845171ec745aa4e/src/daemon/rpmostreed-deployment-utils.c#L227
type rpmOstreeState struct {
Deployments []RpmOstreeDeployment
Transaction *[]string
}

// RpmOstreeDeployment represents a single deployment on a node
Expand Down Expand Up @@ -60,6 +61,7 @@ type imageInspection struct {
// NodeUpdaterClient is an interface describing how to interact with the host
// around content deployment
type NodeUpdaterClient interface {
Initialize() error
GetStatus() (string, error)
GetBootedOSImageURL() (string, string, error)
Rebase(string, string) (bool, error)
Expand All @@ -77,8 +79,14 @@ func NewNodeUpdaterClient() NodeUpdaterClient {
return &RpmOstreeClient{}
}

// GetBootedDeployment returns the current deployment found
func (r *RpmOstreeClient) GetBootedDeployment() (*RpmOstreeDeployment, error) {
// Synchronously invoke rpm-ostree, writing its stdout to our stdout,
// and gathering stderr into a buffer which will be returned in err
// in case of error.
func runRpmOstree(args ...string) error {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we will also need to update Rebase() to call runRpmOstree()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good observation! Thinking about this a bit though, the key thing in this change is that we're invoking the Initialize() method early on. runRpmOstree is just a wrapper that uses the cleaned up subprocess handling in runCmdSync - but it doesn't actually have any of the new special rpm-ostree logic itself.

So while I think you're correct in that this change better matches what lives on master with that suggestion, I don't think it's actually required either.

But regardless - updated!

return runCmdSync("rpm-ostree", args...)
}

func (r *RpmOstreeClient) loadStatus() (*rpmOstreeState, error) {
var rosState rpmOstreeState
output, err := runGetOut("rpm-ostree", "status", "--json")
if err != nil {
Expand All @@ -89,7 +97,53 @@ func (r *RpmOstreeClient) GetBootedDeployment() (*RpmOstreeDeployment, error) {
return nil, errors.Wrapf(err, "failed to parse `rpm-ostree status --json` output (%s)", truncate(string(output), 30))
}

for _, deployment := range rosState.Deployments {
return &rosState, nil
}

func (r *RpmOstreeClient) Initialize() error {
// This replicates https://github.com/coreos/rpm-ostree/pull/2945
// and can be removed when we have a new enough rpm-ostree with
// that PR.
err := runCmdSync("systemctl", "start", "rpm-ostreed")
if err != nil {
// If this fails for some reason, let's dump the unit status
// into our logs to aid future debugging.
cmd := exec.Command("systemctl", "status", "rpm-ostreed")
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
_ = cmd.Run()
return err
}

status, err := r.loadStatus()
if err != nil {
return err
}

// If there's an active transaction when the MCD starts up,
// it's possible that we hit
// https://bugzilla.redhat.com/show_bug.cgi?id=1982389
// This is fixed upstream, but we need a newer RHEL for that,
// so let's just restart the service as a workaround.
if status.Transaction != nil {
glog.Warningf("Detected active transaction during daemon startup, restarting to clear it")
err := runCmdSync("systemctl", "restart", "rpm-ostreed")
if err != nil {
return err
}
}

return nil
}

// GetBootedDeployment returns the current deployment found
func (r *RpmOstreeClient) GetBootedDeployment() (*RpmOstreeDeployment, error) {
status, err := r.loadStatus()
if err != nil {
return nil, err
}

for _, deployment := range status.Deployments {
if deployment.Booted {
deployment := deployment
return &deployment, nil
Expand Down Expand Up @@ -245,10 +299,7 @@ func (r *RpmOstreeClient) Rebase(imgURL, osImageContentDir string) (changed bool
args := []string{"rebase", "--experimental", fmt.Sprintf("%s:%s", repo, ostreeCsum),
"--custom-origin-url", customURL, "--custom-origin-description", "Managed by machine-config-operator"}

var out []byte
if out, err = runGetOut("rpm-ostree", args...); err != nil {
// capture stdout output as well in case of error
err = errors.Wrapf(err, "with stdout output: %v", string(out))
if err = runRpmOstree(args...); err != nil {
return
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/daemon/rpm-ostree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ type RpmOstreeClientMock struct {
GetBootedOSImageURLReturns []GetBootedOSImageURLReturn
}

func (r RpmOstreeClientMock) Initialize() error {
return nil
}

// GetBootedOSImageURL implements a test version of RpmOStreeClients GetBootedOSImageURL.
// It returns an OsImageURL, Version, and Error as defined in GetBootedOSImageURLReturns in order.
func (r RpmOstreeClientMock) GetBootedOSImageURL() (string, string, error) {
Expand Down
16 changes: 16 additions & 0 deletions pkg/daemon/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -1962,6 +1962,22 @@ PENDING=%d`, pendingStateMessageID, pending.GetName(), dn.bootID, isPending))
return logger.CombinedOutput()
}

// Synchronously invoke a command, writing its stdout to our stdout,
// and gathering stderr into a buffer which will be returned in err
// in case of error.
func runCmdSync(cmdName string, args ...string) error {
glog.Infof("Running: %s %s", cmdName, strings.Join(args, " "))
cmd := exec.Command(cmdName, args...)
var stderr bytes.Buffer
cmd.Stdout = os.Stdout
cmd.Stderr = &stderr
if err := cmd.Run(); err != nil {
return fmt.Errorf("error running %s %s: %s: %w", cmdName, strings.Join(args, " "), string(stderr.Bytes()), err)
}

return nil
}

// Log a message to the systemd journal as well as our stdout
func (dn *Daemon) logSystem(format string, a ...interface{}) {
message := fmt.Sprintf(format, a...)
Expand Down