Skip to content

Commit

Permalink
MCO-707: Strip registry transport from os image URL for comparison
Browse files Browse the repository at this point in the history
When comparing os image URLs in or to decide if reboot is needed,
sometimes the URL starts with a prefix (i.e docker:// or registry:), and sometimes
without.  In order to compare correctly, the prefix is removed before
comparison is done.
  • Loading branch information
ori-amizur committed Sep 19, 2023
1 parent 6b6ca7d commit 238165c
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 2 deletions.
23 changes: 21 additions & 2 deletions pkg/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,7 @@ func (dn *Daemon) RunFirstbootCompleteMachineconfig() error {
// Start with an empty config, then add our *booted* osImageURL to
// it, reflecting the current machine state.
oldConfig := canonicalizeEmptyMC(nil)
oldConfig.Spec.OSImageURL = dn.bootedOSImageURL
oldConfig.Spec.OSImageURL = stripRegistryTransports(dn.bootedOSImageURL)

// Setting the Kernel Arguments is for comparison only with the desired MachineConfig.
// The resulting MC should not be for updating node configuration.
Expand Down Expand Up @@ -2295,6 +2295,25 @@ func (dn *Daemon) validateOnDiskStateWithImage(currentConfig *mcfgv1.MachineConf
return nil
}

// stripRegistryTransports removes the registry transport prefixes,
// see https://docs.rs/ostree-ext/latest/ostree_ext/container/enum.Transport.html
func stripRegistryTransports(url string) string {
registryTransports := []string{
"registry:",
"docker://",
}
for shouldUpdate := true; shouldUpdate; {
shouldUpdate = false
for _, rt := range registryTransports {
if strings.HasPrefix(url, rt) {
shouldUpdate = true
url = strings.Replace(url, rt, "", 1)
}
}
}
return url
}

// checkOS determines whether the booted system matches the target
// osImageURL and if not whether we need to take action. This function
// returns `true` if no action is required, which is the case if we're
Expand All @@ -2320,7 +2339,7 @@ func (dn *Daemon) checkOS(osImageURL string) bool {
}
}

return dn.bootedOSImageURL == osImageURL
return stripRegistryTransports(dn.bootedOSImageURL) == osImageURL
}

// Close closes all the connections the node agent has open for it's lifetime
Expand Down
14 changes: 14 additions & 0 deletions pkg/daemon/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -531,3 +531,17 @@ func TestPrepUpdateFromClusterOnDiskDrift(t *testing.T) {
})
}
}

func TestStripRegistryTransports(t *testing.T) {
expectedURL := "quay.io/example/foo@sha256:b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c"
bootedURL := expectedURL
require.Equal(t, expectedURL, stripRegistryTransports(bootedURL))
bootedURL = "docker://" + expectedURL
require.Equal(t, expectedURL, stripRegistryTransports(bootedURL))
bootedURL = "registry:" + expectedURL
require.Equal(t, expectedURL, stripRegistryTransports(bootedURL))
bootedURL = "registry:docker://" + expectedURL
require.Equal(t, expectedURL, stripRegistryTransports(bootedURL))
bootedURL = "registryy:" + expectedURL
require.NotEqual(t, expectedURL, stripRegistryTransports(bootedURL))
}

0 comments on commit 238165c

Please sign in to comment.