Skip to content

Commit

Permalink
Only rebuild plugins if changed (#906)
Browse files Browse the repository at this point in the history 
Update the dockerbuild command to add a label containing the git
revision of the plugin's directory (if there are no changes to the
working directory of the plugin). If we determine that an image exists
for the same revision of the plugin, skip costly rebuilds.

Additionally, update commands to use exec.CommandContext consistently
and stop calling exec.LookPath as the exec package does this
automatically for us if the command doesn't contain path separators.
  • Loading branch information
@pkwarren
pkwarren authored Nov 10, 2023
1 parent 9a5b93b commit 8592949
Show file tree
Hide file tree
Showing 9 changed files with 162 additions and 151 deletions.
4 changes: 2 additions & 2 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ all: build

.PHONY: build
build:
docker buildx inspect "$(DOCKER_BUILDER)" 2> /dev/null || docker buildx create --use --bootstrap --name="$(DOCKER_BUILDER)"
docker buildx inspect "$(DOCKER_BUILDER)" 2> /dev/null || docker buildx create --use --bootstrap --name="$(DOCKER_BUILDER)" > /dev/null
go run ./internal/cmd/dockerbuild -cache-dir "$(DOCKER_CACHE_DIR)" -org "$(DOCKER_ORG)" -- $(DOCKER_BUILD_EXTRA_ARGS) || \
(docker buildx rm "$(DOCKER_BUILDER)"; exit 1)
docker buildx rm "$(DOCKER_BUILDER)"
docker buildx rm "$(DOCKER_BUILDER)" > /dev/null

.PHONY: dockerpush
dockerpush:
Expand Down
33 changes: 32 additions & 1 deletion internal/cmd/dockerbuild/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"log"
"os"
"os/exec"
"path/filepath"
"runtime"
"slices"
Expand Down Expand Up @@ -155,9 +156,14 @@ func (c *command) buildPluginGroup(ctx context.Context, pluginGroup string, plug
pluginCacheDir = filepath.Join(c.cacheDir, pluginIdentity.Owner(), pluginIdentity.Plugin(), pluginToBuild.PluginVersion)
}
}
if !c.shouldBuild(ctx, pluginToBuild) {
log.Println("skipping (up-to-date):", pluginToBuild.Name, pluginToBuild.PluginVersion)
continue
}
log.Println("building:", pluginToBuild.Name, pluginToBuild.PluginVersion)
start := time.Now()
output, err := docker.Build(ctx, pluginToBuild, c.dockerOrg, pluginCacheDir, c.dockerBuildArgs)
imageName := docker.ImageName(pluginToBuild, c.dockerOrg)
output, err := docker.Build(ctx, pluginToBuild, imageName, pluginCacheDir, c.dockerBuildArgs)
if err != nil {
if errors.Is(err, context.Canceled) || strings.Contains(err.Error(), "signal: killed") {
return err
Expand All @@ -175,3 +181,28 @@ func (c *command) buildPluginGroup(ctx context.Context, pluginGroup string, plug
}
return nil
}

func (c *command) shouldBuild(ctx context.Context, plugin *plugin.Plugin) bool {
gitCommit := plugin.GitCommit(ctx)
// There are uncommitted changes to the plugin's directory - we should always rebuild.
if gitCommit == "" {
return true
}
imageName := docker.ImageName(plugin, c.dockerOrg)
cmd := exec.CommandContext(
ctx,
"docker",
"image",
"inspect",
imageName,
"--format",
`{{ index .Config.Labels "org.opencontainers.image.revision" }}`,
)
output, err := cmd.Output()
if err != nil {
// This could be because the image doesn't exist or any other error.
// Err on the side of caution and assume that we should rebuild the image.
return true
}
return strings.TrimSpace(string(output)) != gitCommit
}
69 changes: 23 additions & 46 deletions internal/cmd/fetcher/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"strings"
"time"

"github.com/bufbuild/buf/private/pkg/interrupt"
"go.uber.org/multierr"
"golang.org/x/mod/semver"

Expand All @@ -34,12 +35,13 @@ func main() {
os.Exit(2)
}
root := os.Args[1]
created, err := run(context.Background(), root)
ctx, _ := interrupt.WithCancel(context.Background())
created, err := run(ctx, root)
if err != nil {
_, _ = fmt.Fprintf(os.Stderr, "failed to fetch versions: %v\n", err)
os.Exit(1)
}
if err := postProcessCreatedPlugins(created); err != nil {
if err := postProcessCreatedPlugins(ctx, created); err != nil {
_, _ = fmt.Fprintf(os.Stderr, "failed to run post-processing on plugins: %v\n", err)
os.Exit(1)
}
Expand All @@ -53,28 +55,28 @@ type createdPlugin struct {
newVersion string
}

func postProcessCreatedPlugins(plugins []createdPlugin) error {
func postProcessCreatedPlugins(ctx context.Context, plugins []createdPlugin) error {
if len(plugins) == 0 {
return nil
}
for _, plugin := range plugins {
newPluginRef := fmt.Sprintf("%s/%s:%s", plugin.org, plugin.name, plugin.newVersion)
if err := runGoModTidy(plugin); err != nil {
if err := runGoModTidy(ctx, plugin); err != nil {
return fmt.Errorf("failed to run go mod tidy for %s: %w", newPluginRef, err)
}
if err := recreateNPMPackageLock(plugin); err != nil {
if err := recreateNPMPackageLock(ctx, plugin); err != nil {
return fmt.Errorf("failed to recreate package-lock.json for %s: %w", newPluginRef, err)
}
}
if err := runPluginTests(plugins); err != nil {
if err := runPluginTests(ctx, plugins); err != nil {
return fmt.Errorf("failed to run plugin tests: %w", err)
}
return nil
}

// runGoModTidy runs 'go mod tidy' for plugins (like twirp-go) which don't use modules.
// In order to get more reproducible builds, we check in a go.mod/go.sum file.
func runGoModTidy(plugin createdPlugin) error {
func runGoModTidy(ctx context.Context, plugin createdPlugin) error {
versionDir := filepath.Join(plugin.pluginDir, plugin.newVersion)
goMod := filepath.Join(versionDir, "go.mod")
_, err := os.Stat(goMod)
Expand All @@ -85,24 +87,17 @@ func runGoModTidy(plugin createdPlugin) error {
// no go.mod/go.sum to update
return nil
}
goPath, err := exec.LookPath("go")
if err != nil {
return err
}
log.Printf("running go mod tidy for %s/%s:%s", plugin.org, plugin.name, plugin.newVersion)
cmd := exec.Cmd{
Path: goPath,
Args: []string{goPath, "mod", "tidy"},
Dir: versionDir,
Stdout: os.Stdout,
Stderr: os.Stderr,
}
cmd := exec.CommandContext(ctx, "go", "mod", "tidy")
cmd.Dir = versionDir
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
return cmd.Run()
}

// recreateNPMPackageLock will remove an existing package-lock.json file and recreate it.
// This will ensure that we correctly resolve any updated versions in package.json.
func recreateNPMPackageLock(plugin createdPlugin) error {
func recreateNPMPackageLock(ctx context.Context, plugin createdPlugin) error {
versionDir := filepath.Join(plugin.pluginDir, plugin.newVersion)
npmPackageLock := filepath.Join(versionDir, "package-lock.json")
_, err := os.Stat(npmPackageLock)
Expand All @@ -116,49 +111,31 @@ func recreateNPMPackageLock(plugin createdPlugin) error {
if err := os.Remove(npmPackageLock); err != nil {
return err
}
npmPath, err := exec.LookPath("npm")
if err != nil {
return err
}
log.Printf("recreating package-lock.json for %s/%s:%s", plugin.org, plugin.name, plugin.newVersion)
cmd := exec.Cmd{
Path: npmPath,
Args: []string{npmPath, "install"},
Dir: versionDir,
Stdout: os.Stdout,
Stderr: os.Stderr,
}
cmd := exec.CommandContext(ctx, "npm", "install")
cmd.Dir = versionDir
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
return cmd.Run()
}

// runPluginTests runs 'make test PLUGINS="org/name:v<new>"' in order to generate plugin.sum files.
func runPluginTests(plugins []createdPlugin) error {
func runPluginTests(ctx context.Context, plugins []createdPlugin) error {
pluginsEnv := make([]string, 0, len(plugins))
for _, plugin := range plugins {
pluginsEnv = append(pluginsEnv, fmt.Sprintf("%s/%s:%s", plugin.org, plugin.name, plugin.newVersion))
}
makePath, err := exec.LookPath("make")
if err != nil {
return err
}
env := os.Environ()
env = append(env, "ALLOW_EMPTY_PLUGIN_SUM=true")
cmd := exec.Cmd{
Path: makePath,
Args: []string{
makePath,
"test",
fmt.Sprintf("PLUGINS=%s", strings.Join(pluginsEnv, ",")),
},
Env: env,
Stdout: os.Stdout,
Stderr: os.Stderr,
}
start := time.Now()
log.Printf("starting running tests for %d plugins", len(plugins))
defer func() {
log.Printf("finished running tests in: %.2fs", time.Since(start).Seconds())
}()
cmd := exec.CommandContext(ctx, "make", "test", fmt.Sprintf("PLUGINS=%s", strings.Join(pluginsEnv, ","))) //nolint:gosec
cmd.Env = env
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
return cmd.Run()
}

Expand Down
52 changes: 18 additions & 34 deletions internal/cmd/release/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"time"

"aead.dev/minisign"
"github.com/bufbuild/buf/private/pkg/interrupt"
githubkeychain "github.com/google/go-containerregistry/pkg/authn/github"
"github.com/google/go-containerregistry/pkg/name"
"github.com/google/go-containerregistry/pkg/v1/remote"
Expand Down Expand Up @@ -80,6 +81,8 @@ type command struct {
}

func (c *command) run() error {
ctx, cancel := interrupt.WithCancel(context.Background())
defer cancel()
// Create temporary directory
tmpDir, err := os.MkdirTemp("", "plugins-release")
if err != nil {
Expand All @@ -94,8 +97,6 @@ func (c *command) run() error {
log.Printf("failed to remove %q: %v", tmpDir, err)
}
}()

ctx := context.Background()
client := release.NewClient(ctx)
latestRelease, err := client.GetLatestRelease(ctx, c.githubReleaseOwner, release.GithubRepoPlugins)
if err != nil && !errors.Is(err, release.ErrNotFound) {
Expand Down Expand Up @@ -128,7 +129,7 @@ func (c *command) run() error {
return fmt.Errorf("failed to determine next release name: %w", err)
}

plugins, err := c.calculateNewReleasePlugins(releases, releaseName, now, tmpDir)
plugins, err := c.calculateNewReleasePlugins(ctx, releases, releaseName, now, tmpDir)
if err != nil {
return fmt.Errorf("failed to calculate new release contents: %w", err)
}
Expand Down Expand Up @@ -166,7 +167,7 @@ func (c *command) run() error {
return nil
}

func (c *command) calculateNewReleasePlugins(currentRelease *release.PluginReleases, releaseName string, now time.Time, tmpDir string) (
func (c *command) calculateNewReleasePlugins(ctx context.Context, currentRelease *release.PluginReleases, releaseName string, now time.Time, tmpDir string) (
[]release.PluginRelease, error,
) {
pluginNameVersionToRelease := make(map[pluginNameVersion]release.PluginRelease, len(currentRelease.Releases))
Expand Down Expand Up @@ -200,7 +201,7 @@ func (c *command) calculateNewReleasePlugins(currentRelease *release.PluginRelea
// Found existing release - only rebuild if changed image digest or buf.plugin.yaml digest
if pluginRelease.ImageID != imageID || pluginRelease.PluginYAMLDigest != pluginYamlDigest {
downloadURL := c.pluginDownloadURL(plugin, releaseName)
zipDigest, err := createPluginZip(tmpDir, plugin, registryImage, imageID)
zipDigest, err := createPluginZip(ctx, tmpDir, plugin, registryImage, imageID)
if err != nil {
return err
}
Expand Down Expand Up @@ -409,8 +410,8 @@ func signPluginReleases(dir string, privateKey minisign.PrivateKey) error {
return nil
}

func createPluginZip(basedir string, plugin *plugin.Plugin, registryImage string, imageID string) (string, error) {
if err := pullImage(registryImage); err != nil {
func createPluginZip(ctx context.Context, basedir string, plugin *plugin.Plugin, registryImage string, imageID string) (string, error) {
if err := pullImage(ctx, registryImage); err != nil {
return "", err
}
zipName := pluginZipName(plugin)
Expand All @@ -423,7 +424,7 @@ func createPluginZip(basedir string, plugin *plugin.Plugin, registryImage string
log.Printf("failed to remove %q: %v", pluginTempDir, err)
}
}()
if err := saveImageToDir(imageID, pluginTempDir); err != nil {
if err := saveImageToDir(ctx, imageID, pluginTempDir); err != nil {
return "", err
}
log.Printf("creating %s", zipName)
Expand Down Expand Up @@ -480,11 +481,8 @@ func addFileToZip(zipWriter *zip.Writer, path string) error {
return nil
}

func saveImageToDir(imageRef string, dir string) error {
cmd, err := dockerCmd("save", imageRef, "-o", "image.tar")
if err != nil {
return err
}
func saveImageToDir(ctx context.Context, imageRef string, dir string) error {
cmd := dockerCmd(ctx, "save", imageRef, "-o", "image.tar")
cmd.Dir = dir
return cmd.Run()
}
Expand All @@ -504,30 +502,16 @@ func createPluginReleases(dir string, plugins []release.PluginRelease) error {
return encoder.Encode(&release.PluginReleases{Releases: plugins})
}

func pullImage(name string) error {
cmd, err := dockerCmd("pull", name)
if err != nil {
return err
}
func pullImage(ctx context.Context, name string) error {
log.Printf("pulling image: %s", name)
return cmd.Run()
return dockerCmd(ctx, "pull", name).Run()
}

func dockerCmd(command string, args ...string) (*exec.Cmd, error) {
dockerPath, err := exec.LookPath("docker")
if err != nil {
return nil, err
}
cmd := &exec.Cmd{
Path: dockerPath,
Args: append([]string{
dockerPath,
command,
}, args...),
Stdout: os.Stdout,
Stderr: os.Stderr,
}
return cmd, nil
func dockerCmd(ctx context.Context, command string, args ...string) *exec.Cmd {
cmd := exec.CommandContext(ctx, "docker", append([]string{command}, args...)...) //nolint:gosec
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
return cmd
}

func calculateNextRelease(now time.Time, latestRelease *github.RepositoryRelease) (string, error) {
Expand Down
Loading

0 comments on commit 8592949

Please sign in to comment.