cos-dkms: Load configs when instantiating Packages
This significantly reduces the number of times configs have to be
loaded, which results in a noticeable speedup, especially when loading
from a GCS bucket. As a crude measurement, this reduces the time to run
the Test*Transition unit tests from ~3 seconds to ~1 second.
BUG=None
TEST=presubmit
Change-Id: I73ac6375cdedb44dcfc6deac851b914b0b9d0623
Reviewed-on: https://cos-review.googlesource.com/c/cos/tools/+/111403
Reviewed-by: Angel Adetula <angeladetula@google.com>
Cloud-Build: GCB Service account <228075978874@cloudbuild.gserviceaccount.com>
Tested-by: Kevin Berry <kpberry@google.com>
diff --git a/src/cmd/cos_dkms/actions/BUILD.bazel b/src/cmd/cos_dkms/actions/BUILD.bazel
index 52ce01f..a793cbf 100644
--- a/src/cmd/cos_dkms/actions/BUILD.bazel
+++ b/src/cmd/cos_dkms/actions/BUILD.bazel
@@ -18,6 +18,7 @@
deps = [
"//src/pkg/cos",
"//src/pkg/dkms",
+ "//src/pkg/fs",
"//src/pkg/gcs",
"//src/pkg/modules",
"//src/pkg/utils",
diff --git a/src/cmd/cos_dkms/actions/args.go b/src/cmd/cos_dkms/actions/args.go
index bb5dcdc..5accab9 100644
--- a/src/cmd/cos_dkms/actions/args.go
+++ b/src/cmd/cos_dkms/actions/args.go
@@ -12,6 +12,7 @@
"cloud.google.com/go/storage"
"cos.googlesource.com/cos/tools.git/src/pkg/cos"
"cos.googlesource.com/cos/tools.git/src/pkg/dkms"
+ "cos.googlesource.com/cos/tools.git/src/pkg/fs"
"cos.googlesource.com/cos/tools.git/src/pkg/gcs"
"cos.googlesource.com/cos/tools.git/src/pkg/utils"
"github.com/golang/glog"
@@ -162,6 +163,15 @@
pkg.Trees.KernelModules = pkg.Trees.Install
}
+ if fs.IsDir(pkg.SourceDir()) {
+ config, err := dkms.LoadConfig(pkg)
+ if err != nil {
+ glog.Warningf("error loading dkms config: %v", err)
+ } else {
+ pkg.Config = config
+ }
+ }
+
return nil
}
diff --git a/src/pkg/dkms/add.go b/src/pkg/dkms/add.go
index 82917f1..f723070 100644
--- a/src/pkg/dkms/add.go
+++ b/src/pkg/dkms/add.go
@@ -12,7 +12,9 @@
"github.com/golang/glog"
)
-// Add adds a package to the DKMS source tree.
+// Add adds a package to the DKMS source tree and loads its config.
+//
+// After calling this function, the package's Config will be set.
//
// This checks the user source tree (by default, /usr/src/)
// for the package source files. If the sources don't exist there,
@@ -94,6 +96,7 @@
if err != nil {
return err
}
+ pkg.Config = config
if config.PostAdd != "" {
if err := utils.RunCommandString(dkmsTreeSourceDir, config.PostAdd); err != nil {
@@ -166,28 +169,15 @@
return nil
}
-// IsAdded checks if a package has been added to the DKMS tree and if it
-// has a valid dkms.conf file.
-// Note that an empty or absent dkms.conf is considered valid, since default
-// values can be used for all settings.
+// IsAdded checks if a package has been added to the DKMS tree and its
+// config has been loaded.
func IsAdded(pkg *Package) bool {
- configPath := pkg.ConfigPath()
- if !fs.IsFile(configPath) {
- glog.Infof("could not find config at %s\n", configPath)
- return fs.IsDir(pkg.SourceDir())
- }
-
- // make sure the config parses without error, but ignore the contents
- if _, err := LoadConfig(pkg); err != nil {
- glog.Warningf("dkms.conf found, but invalid: %v", err)
- return false
- }
-
- return true
+ return fs.IsDir(pkg.SourceDir()) && pkg.Config != nil
}
// IsAddedInCache checks if a package has been added to the DKMS tree in the
// cache and if it has a valid dkms.conf file.
+//
// Note that an empty or absent dkms.conf is considered valid, since default
// values can be used for all settings.
func IsAddedInCache(ctx context.Context, pkg *Package, cache *gcs.GCSBucket) bool {
@@ -209,8 +199,13 @@
return sourceExists
}
- // make sure the config parses without error, but ignore the contents
- if _, err := LoadConfig(pkg); err != nil {
+ // make sure the remote config parses without error, but ignore the contents
+ config, err := cache.DownloadTextObject(ctx, configPath)
+ if err != nil {
+ return false
+ }
+
+ if _, err := ParseConfig(config, pkg); err != nil {
glog.Warningf("dkms.conf found, but invalid: %v", err)
return false
}
diff --git a/src/pkg/dkms/build.go b/src/pkg/dkms/build.go
index 4bb006a..25215c8 100644
--- a/src/pkg/dkms/build.go
+++ b/src/pkg/dkms/build.go
@@ -41,6 +41,11 @@
// used for compiling the COS kernel and append them to the MAKE command. See
// DefaultMakeVariables for more detail.
func Build(pkg *Package, options *Options) error {
+ // This must happen first in order to set the package config.
+ if err := Add(pkg, options); err != nil {
+ return err
+ }
+
if IsBuilt(pkg) {
if !options.Force {
glog.Info("package already built; skipping build")
@@ -52,18 +57,9 @@
}
}
- if err := Add(pkg, options); err != nil {
- return err
- }
-
glog.Infof("building package %s-%s", pkg.Name, pkg.Version)
- config, err := LoadConfig(pkg)
- if err != nil {
- return err
- }
-
- if config.PreBuild != "" {
- if err := utils.RunCommandString(pkg.BuildDir(), config.PreBuild); err != nil {
+ if pkg.Config.PreBuild != "" {
+ if err := utils.RunCommandString(pkg.BuildDir(), pkg.Config.PreBuild); err != nil {
return fmt.Errorf("failed to run pre-build script: %v", err)
}
}
@@ -82,7 +78,7 @@
return err
}
- for _, patch := range config.Patches {
+ for _, patch := range pkg.Config.Patches {
patchPath := path.Join(buildDir, "patches", patch)
if err := ApplyPatch(buildDir, patchPath); err != nil {
return fmt.Errorf("could not apply patch: %v", err)
@@ -91,24 +87,25 @@
makeVariables := options.MakeVariables
if makeVariables == "cos-default" {
+ var err error
makeVariables, err = DefaultMakeVariables(pkg)
if err != nil {
return err
}
}
- makeCommand := fmt.Sprintf("%s %s", config.MakeCommand, makeVariables)
+ makeCommand := fmt.Sprintf("%s %s", pkg.Config.MakeCommand, makeVariables)
makeCommand = strings.Trim(makeCommand, " ")
if err := utils.RunCommandString(buildDir, makeCommand); err != nil {
return fmt.Errorf("failed to compile modules: %v", err)
}
- if err := SignModules(config.Modules, options.PrivateKeyPath, options.CertificatePath, options.Hash); err != nil {
+ if err := SignModules(pkg.Config.Modules, options.PrivateKeyPath, options.CertificatePath, options.Hash); err != nil {
glog.Warningf("skipping module signing for package (%s): %v", pkg.Name, err)
}
- if config.PostBuild != "" {
- if err := utils.RunCommandString(buildDir, config.PostBuild); err != nil {
+ if pkg.Config.PostBuild != "" {
+ if err := utils.RunCommandString(buildDir, pkg.Config.PostBuild); err != nil {
return fmt.Errorf("failed to run post-build script: %v", err)
}
}
@@ -245,14 +242,9 @@
return err
}
- config, err := LoadConfig(pkg)
- if err != nil {
- return err
- }
-
// Get the list of modules which are not available locally
var moduleDownloads []gcs.ObjectDownload
- for _, module := range config.Modules {
+ for _, module := range pkg.Config.Modules {
builtPath := module.BuiltPath()
if !fs.IsFile(builtPath) {
glog.Infof("could not find compiled module %s locally", module.BuiltName)
@@ -277,15 +269,13 @@
}
}
- // If all modules were downloaded successfully, this should be a no-op.
- err = Build(pkg, options)
- if err != nil {
+ if err := Build(pkg, options); err != nil {
return err
}
// Upload the modules to the cache after they have all been built.
if options.Upload && !IsBuiltInCache(ctx, pkg, cache) {
- for _, module := range config.Modules {
+ for _, module := range pkg.Config.Modules {
builtPath := module.BuiltPath()
cacheBuiltPath := module.CacheBuiltPath()
err := cache.UploadObjectFromFile(ctx, builtPath, cacheBuiltPath)
@@ -293,20 +283,18 @@
return err
}
}
- return nil
}
return nil
}
-// isBuilt returns whether or not all of a package's modules have been built.
-// If the package has an invalid dkms.conf, this returns false.
-func isBuilt(pkg *Package) bool {
- config, err := LoadConfig(pkg)
- if err != nil {
+// IsBuilt returns whether or not all of a package's modules have been built.
+func IsBuilt(pkg *Package) bool {
+ if pkg.Config == nil {
return false
}
- for _, module := range config.Modules {
+
+ for _, module := range pkg.Config.Modules {
if !fs.IsFile(module.BuiltPath()) {
return false
}
@@ -315,22 +303,16 @@
return true
}
-// IsBuilt returns whether or not a package has been added to the DKMS source
-// tree and all of its modules have been built.
-func IsBuilt(pkg *Package) bool {
- return isBuilt(pkg) && IsAdded(pkg)
-}
-
// IsBuiltInCache returns whether or not all of a package's built modules are
// present in a cache.
-// If the package has an invalid dkms.conf, this returns false.
+//
+// If the package does not have its Config set, this returns false.
func IsBuiltInCache(ctx context.Context, pkg *Package, cache *gcs.GCSBucket) bool {
- config, err := LoadConfig(pkg)
- if err != nil {
+ if pkg.Config == nil {
return false
}
- for _, module := range config.Modules {
+ for _, module := range pkg.Config.Modules {
builtPath := module.CacheBuiltPath()
exists, err := cache.Exists(ctx, builtPath)
if !exists || err != nil {
diff --git a/src/pkg/dkms/install.go b/src/pkg/dkms/install.go
index f1629ac..d88dc38 100644
--- a/src/pkg/dkms/install.go
+++ b/src/pkg/dkms/install.go
@@ -28,6 +28,11 @@
// the module after it is copied to the install tree to insert it into the
// running kernel.
func Install(pkg *Package, options *Options) error {
+ // This must happen first in order to set the package config.
+ if err := Build(pkg, options); err != nil {
+ return err
+ }
+
if IsInstalled(pkg) {
if !options.Force {
glog.Info("package already installed; skipping installation")
@@ -39,27 +44,18 @@
}
}
- if err := Build(pkg, options); err != nil {
- return err
- }
-
glog.Infof("installing package %s-%s", pkg.Name, pkg.Version)
- config, err := LoadConfig(pkg)
- if err != nil {
- return err
- }
-
- if config.PreInstall != "" {
- if err := utils.RunCommandString(pkg.BuildDir(), config.PreInstall); err != nil {
+ if pkg.Config.PreInstall != "" {
+ if err := utils.RunCommandString(pkg.BuildDir(), pkg.Config.PreInstall); err != nil {
return fmt.Errorf("failed to run pre-install script: %v", err)
}
}
var modulesToInstall []Module
if options.ForceVersionOverride {
- modulesToInstall = config.Modules
+ modulesToInstall = pkg.Config.Modules
} else {
- modules, err := ModulesToInstall(config.Modules, pkg.Trees.Install)
+ modules, err := ModulesToInstall(pkg.Config.Modules, pkg.Trees.Install)
if err != nil {
return err
}
@@ -116,13 +112,13 @@
}
}
- if err := ModprobeModules(config.Modules); err != nil {
+ if err := ModprobeModules(pkg.Config.Modules); err != nil {
return err
}
}
- if config.PostInstall != "" {
- if err := utils.RunCommandString(pkg.BuildDir(), config.PostInstall); err != nil {
+ if pkg.Config.PostInstall != "" {
+ if err := utils.RunCommandString(pkg.BuildDir(), pkg.Config.PostInstall); err != nil {
return fmt.Errorf("failed to run post-install script: %v", err)
}
}
@@ -193,15 +189,16 @@
return result, nil
}
-// isInstalled returns whether or not all of a package's modules have been
+// IsInstalled returns whether or not all of a package's modules have been
// installed.
-// If the package has an invalid dkms.conf, this returns false.
-func isInstalled(pkg *Package) bool {
- config, err := LoadConfig(pkg)
- if err != nil {
+//
+// If the package does not have its config set, this returns false.
+func IsInstalled(pkg *Package) bool {
+ if pkg.Config == nil {
return false
}
- for _, module := range config.Modules {
+
+ for _, module := range pkg.Config.Modules {
if !fs.IsFile(module.DestPath()) {
return false
}
@@ -210,8 +207,3 @@
return true
}
-// IsInstalled returns whether or not a package has been built in the DKMS
-// build tree and all of its modules have been installed in the install tree.
-func IsInstalled(pkg *Package) bool {
- return isInstalled(pkg) && IsBuilt(pkg)
-}
diff --git a/src/pkg/dkms/package.go b/src/pkg/dkms/package.go
index f9c9fea..934901c 100644
--- a/src/pkg/dkms/package.go
+++ b/src/pkg/dkms/package.go
@@ -60,6 +60,8 @@
// This is the collection of trees which will be used as the base directories
// for the paths related to this package.
Trees *Trees
+ // This is the configuration for the package, as specified in its dkms.conf.
+ Config *Config
}
// SourceTreeDir returns the path to the package within the user's source tree.
diff --git a/src/pkg/dkms/remove.go b/src/pkg/dkms/remove.go
index 9bbfff9..57e417c 100644
--- a/src/pkg/dkms/remove.go
+++ b/src/pkg/dkms/remove.go
@@ -13,21 +13,12 @@
// Remove removes a package from the DKMS source tree.
// This will also unbuild and uninstall the package, if applicable.
func Remove(pkg *Package) error {
- if isBuilt(pkg) {
- if err := Unbuild(pkg); err != nil {
- return err
- }
- }
-
if !IsAdded(pkg) {
glog.Info("module is not yet added; skipping removal from source tree")
return nil
}
- // We must load the config before the package is removed in order to use
- // the correct post-remove script, if applicable
- config, err := LoadConfig(pkg)
- if err != nil {
+ if err := Unbuild(pkg); err != nil {
return err
}
@@ -37,8 +28,8 @@
return fmt.Errorf("failed to remove package sources: %v", err)
}
- if config.PostRemove != "" {
- if err := utils.RunCommandString(sourceDir, config.PostRemove); err != nil {
+ if pkg.Config.PostRemove != "" {
+ if err := utils.RunCommandString(sourceDir, pkg.Config.PostRemove); err != nil {
return fmt.Errorf("failed to run post-remove script: %v", err)
}
}
@@ -51,10 +42,8 @@
// This will also unbuild the package locally and in the cache, if applicable.
// If the package is installed, this will uninstall it.
func CachedRemove(ctx context.Context, pkg *Package, cache *gcs.GCSBucket) error {
- if IsBuilt(pkg) {
- if err := CachedUnbuild(ctx, pkg, cache); err != nil {
- return err
- }
+ if err := CachedUnbuild(ctx, pkg, cache); err != nil {
+ return err
}
if err := Remove(pkg); err != nil {
diff --git a/src/pkg/dkms/status.go b/src/pkg/dkms/status.go
index e93902d..93f62b2 100644
--- a/src/pkg/dkms/status.go
+++ b/src/pkg/dkms/status.go
@@ -8,16 +8,37 @@
// Status returns the status of a DKMS package.
func Status(pkg *Package) PackageStatus {
- if IsInstalled(pkg) {
+ pkgCopy := pkg
+ if pkg.Config == nil {
+ config, err := LoadConfig(pkg)
+ if err != nil {
+ return Broken
+ }
+ pkgCopy.Config = config
+ }
+
+ added := IsAdded(pkgCopy)
+ if !added {
+ return Broken
+ }
+
+ built := IsBuilt(pkgCopy)
+ installed := IsInstalled(pkgCopy)
+
+ // The cases below are the only valid ones...
+ if added && built && installed {
return Installed
}
- if IsBuilt(pkg) {
+
+ if added && built && !installed {
return Built
}
- if IsAdded(pkg) {
+
+ if added && !built && !installed {
return Added
}
+ // ... any other case is considered broken.
return Broken
}
@@ -25,13 +46,32 @@
// If a module is built or added only in the cache, the module will have a
// status of Built or Added, respectively.
func CachedStatus(ctx context.Context, pkg *Package, cache *gcs.GCSBucket) PackageStatus {
- if IsInstalled(pkg) {
- return Installed
+ status := Status(pkg)
+
+ // No need to check the cache in this case.
+ if status == Installed || status == Built {
+ return status
}
- if IsBuilt(pkg) || IsBuiltInCache(ctx, pkg, cache) {
+
+ pkgCopy := pkg
+ if pkg.Config == nil {
+ configBytes, err := cache.DownloadTextObject(ctx, pkg.CacheConfigPath())
+ if err != nil {
+ return Broken
+ }
+
+ config, err := ParseConfig(configBytes, pkg)
+ if err != nil {
+ return Broken
+ }
+ pkgCopy.Config = config
+ }
+
+ if IsBuiltInCache(ctx, pkgCopy, cache) {
return Built
}
- if IsAdded(pkg) || IsAddedInCache(ctx, pkg, cache) {
+
+ if IsAddedInCache(ctx, pkgCopy, cache) {
return Added
}
diff --git a/src/pkg/dkms/unbuild.go b/src/pkg/dkms/unbuild.go
index 1cf5f11..1785ddc 100644
--- a/src/pkg/dkms/unbuild.go
+++ b/src/pkg/dkms/unbuild.go
@@ -15,24 +15,16 @@
// This removes the compiled modules from the build directory, then calls
// the MAKE clean command for the package from the package's build directory.
func Unbuild(pkg *Package) error {
- var err error
- if isInstalled(pkg) {
- if err := Uninstall(pkg); err != nil {
- return err
- }
- }
-
- if !isBuilt(pkg) {
+ if !IsBuilt(pkg) {
glog.Info("module is not built; skipping removal from build tree")
return nil
}
- config, err := LoadConfig(pkg)
- if err != nil {
+ if err := Uninstall(pkg); err != nil {
return err
}
- for _, module := range config.Modules {
+ for _, module := range pkg.Config.Modules {
builtModulePath := module.BuiltPath()
glog.Info("removing built module", builtModulePath)
if err := os.Remove(builtModulePath); err != nil {
@@ -40,7 +32,7 @@
}
}
- if err := clean(pkg.BuildDir(), config.Clean); err != nil {
+ if err := clean(pkg.BuildDir(), pkg.Config.Clean); err != nil {
return fmt.Errorf("failed to clean modules: %v", err)
}
@@ -72,12 +64,7 @@
}
if IsBuiltInCache(ctx, pkg, cache) {
- config, err := LoadConfig(pkg)
- if err != nil {
- return err
- }
-
- for _, module := range config.Modules {
+ for _, module := range pkg.Config.Modules {
builtModulePath := module.CacheBuiltPath()
if err := cache.DeleteObject(ctx, builtModulePath); err != nil {
return err
diff --git a/src/pkg/dkms/uninstall.go b/src/pkg/dkms/uninstall.go
index 5183754..4260be5 100644
--- a/src/pkg/dkms/uninstall.go
+++ b/src/pkg/dkms/uninstall.go
@@ -14,12 +14,7 @@
return nil
}
- config, err := LoadConfig(pkg)
- if err != nil {
- return err
- }
-
- for _, module := range config.Modules {
+ for _, module := range pkg.Config.Modules {
destModulePath := module.DestPath()
glog.Info("removing installed module", destModulePath)
if err := os.Remove(destModulePath); err != nil {
