Skip to content

Commit

Permalink
Merge branch 'GoogleContainerTools:main' into bazel-platforms
Browse files Browse the repository at this point in the history
  • Loading branch information
aran committed Feb 23, 2024
2 parents f75b1f8 + eeaf001 commit 0137e1e
Show file tree
Hide file tree
Showing 21 changed files with 410 additions and 148 deletions.
2 changes: 1 addition & 1 deletion deploy/skaffold/Dockerfile.deps
Original file line number Diff line number Diff line change
Expand Up @@ -163,5 +163,5 @@ RUN apt-get update && apt-get install --no-install-recommends --no-install-sugge
jq \
apt-transport-https && \
rm -rf /var/lib/apt/lists/*
COPY --from=golang:1.21.0 /usr/local/go /usr/local/go
COPY --from=golang:1.21.6 /usr/local/go /usr/local/go
ENV PATH /usr/local/go/bin:/root/go/bin:$PATH
2 changes: 1 addition & 1 deletion deploy/skaffold/Dockerfile.deps.slim
Original file line number Diff line number Diff line change
Expand Up @@ -104,5 +104,5 @@ RUN apt-get update && apt-get install --no-install-recommends --no-install-sugge
jq \
apt-transport-https && \
rm -rf /var/lib/apt/lists/*
COPY --from=golang:1.21.0 /usr/local/go /usr/local/go
COPY --from=golang:1.21.6 /usr/local/go /usr/local/go
ENV PATH /usr/local/go/bin:/root/go/bin:$PATH
64 changes: 64 additions & 0 deletions docs-v2/content/en/schemas/v4beta10.json
Original file line number Diff line number Diff line change
Expand Up @@ -1319,6 +1319,11 @@
"description": "describes a remote git repository containing the required configs.",
"x-intellij-html-description": "describes a remote git repository containing the required configs."
},
"googleCloudBuildRepoV2": {
"$ref": "#/definitions/GoogleCloudBuildRepoV2Info",
"description": "describes a [Google Cloud Build repository (2nd gen)](https://cloud.google.com/build/docs/repositories#repositories_2nd_gen) that points to a repo with the required configs.",
"x-intellij-html-description": "describes a <a href=\"https://cloud.google.com/build/docs/repositories#repositories_2nd_gen\">Google Cloud Build repository (2nd gen)</a> that points to a repo with the required configs."
},
"googleCloudStorage": {
"$ref": "#/definitions/GoogleCloudStorageInfo",
"description": "describes remote Google Cloud Storage objects containing the required configs.",
Expand All @@ -1335,6 +1340,7 @@
"path",
"git",
"googleCloudStorage",
"googleCloudBuildRepoV2",
"activeProfiles"
],
"additionalProperties": false,
Expand Down Expand Up @@ -2133,6 +2139,64 @@
"description": "*beta* describes how to do a remote build on [Google Cloud Build](https://cloud.google.com/cloud-build/docs/). Docker and Jib artifacts can be built on Cloud Build. The `projectId` needs to be provided and the currently logged in user should be given permissions to trigger new builds.",
"x-intellij-html-description": "<em>beta</em> describes how to do a remote build on <a href=\"https://cloud.google.com/cloud-build/docs/\">Google Cloud Build</a>. Docker and Jib artifacts can be built on Cloud Build. The <code>projectId</code> needs to be provided and the currently logged in user should be given permissions to trigger new builds."
},
"GoogleCloudBuildRepoV2Info": {
"required": [
"projectID",
"region",
"connection",
"repo"
],
"properties": {
"connection": {
"type": "string",
"description": "name of the GCB repository connection associated with the repo.",
"x-intellij-html-description": "name of the GCB repository connection associated with the repo."
},
"path": {
"type": "string",
"description": "relative path from the repo root to the skaffold configuration file. eg. `getting-started/skaffold.yaml`.",
"x-intellij-html-description": "relative path from the repo root to the skaffold configuration file. eg. <code>getting-started/skaffold.yaml</code>."
},
"projectID": {
"type": "string",
"description": "ID of the GCP project where the repository is configured.",
"x-intellij-html-description": "ID of the GCP project where the repository is configured."
},
"ref": {
"type": "string",
"description": "git ref the repo should be cloned from. e.g. `master` or `main`.",
"x-intellij-html-description": "git ref the repo should be cloned from. e.g. <code>master</code> or <code>main</code>."
},
"region": {
"type": "string",
"description": "GCP region where the repository is configured.",
"x-intellij-html-description": "GCP region where the repository is configured."
},
"repo": {
"type": "string",
"description": "name of repository under the given connection.",
"x-intellij-html-description": "name of repository under the given connection."
},
"sync": {
"type": "boolean",
"description": "when set to `true` will reset the cached repository to the latest commit from remote on every run. To use the cached repository with uncommitted changes or unpushed commits, it needs to be set to `false`.",
"x-intellij-html-description": "when set to <code>true</code> will reset the cached repository to the latest commit from remote on every run. To use the cached repository with uncommitted changes or unpushed commits, it needs to be set to <code>false</code>."
}
},
"preferredOrder": [
"projectID",
"region",
"connection",
"repo",
"path",
"ref",
"sync"
],
"additionalProperties": false,
"type": "object",
"description": "contains information on the origin of skaffold configurations cloned from Google Cloud Build repository (2nd gen).",
"x-intellij-html-description": "contains information on the origin of skaffold configurations cloned from Google Cloud Build repository (2nd gen)."
},
"GoogleCloudStorageInfo": {
"properties": {
"path": {
Expand Down
2 changes: 1 addition & 1 deletion examples/buildpacks-python/requirements.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Flask==3.0.1
Flask==3.0.2
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ require (
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/oklog/ulid v1.3.1 // indirect
github.com/onsi/ginkgo v1.16.5 // indirect
github.com/opencontainers/runc v1.1.7 // indirect
github.com/opencontainers/runc v1.1.12 // indirect
github.com/opencontainers/selinux v1.11.0 // indirect
github.com/pelletier/go-toml v1.9.5 // indirect
github.com/pelletier/go-toml/v2 v2.0.8 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -928,8 +928,8 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8
github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
github.com/opencontainers/image-spec v1.1.0-rc2.0.20221005185240-3a7f492d3f1b h1:YWuSjZCQAPM8UUBLkYUk1e+rZcvWHJmFb6i6rM44Xs8=
github.com/opencontainers/image-spec v1.1.0-rc2.0.20221005185240-3a7f492d3f1b/go.mod h1:3OVijpioIKYWTqjiG0zfF6wvoJ4fAXGbjdZuI2NgsRQ=
github.com/opencontainers/runc v1.1.7 h1:y2EZDS8sNng4Ksf0GUYNhKbTShZJPJg1FiXJNH/uoCk=
github.com/opencontainers/runc v1.1.7/go.mod h1:CbUumNnWCuTGFukNXahoo/RFBZvDAgRh/smNYNOhA50=
github.com/opencontainers/runc v1.1.12 h1:BOIssBaW1La0/qbNZHXOOa71dZfZEQOzW7dqQf3phss=
github.com/opencontainers/runc v1.1.12/go.mod h1:S+lQwSfncpBha7XTy/5lBwWgm5+y5Ma/O44Ekby9FK8=
github.com/opencontainers/selinux v1.11.0 h1:+5Zbo97w3Lbmb3PeqQtpmTkMwsW5nRI3YaLpt7tQ7oU=
github.com/opencontainers/selinux v1.11.0/go.mod h1:E5dMC3VPuVvVHDYmi78qvhJp8+M586T4DlDRYpFkyec=
github.com/openzipkin/zipkin-go v0.1.1/go.mod h1:NtoC/o8u3JlF1lSlyPNswIbeQH9bJTmOf0Erfk+hxe8=
Expand Down
2 changes: 1 addition & 1 deletion integration/build_dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ func TestBuildDependenciesCache(t *testing.T) {
// The test then triggers another build and verifies that the images in `rebuilt` were built
// (e.g., the changed images and their dependents), and that the other images were found in the artifact cache.
// It runs the profile `concurrency-0` which builds with maximum concurrency.
MarkIntegrationTest(t, CanRunWithoutGcp)
tests := []struct {
description string
change []int
Expand Down Expand Up @@ -137,6 +136,7 @@ func TestBuildDependenciesCache(t *testing.T) {

for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
MarkIntegrationTest(t, CanRunWithoutGcp)
// modify file `foo` to invalidate cache for target artifacts
for _, i := range test.change {
Run(t, fmt.Sprintf("testdata/build-dependencies/app%d", i), "sh", "-c", fmt.Sprintf("echo %s > foo", uuid.New().String()))
Expand Down
4 changes: 3 additions & 1 deletion integration/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"strings"
"testing"

"github.com/google/uuid"

"github.com/GoogleContainerTools/skaffold/v2/cmd/skaffold/app/flags"
"github.com/GoogleContainerTools/skaffold/v2/integration/skaffold"
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util"
Expand All @@ -38,7 +40,7 @@ func TestBuildDeploy(t *testing.T) {

ns, client := SetupNamespace(t)

outputBytes := skaffold.Build("--quiet", "--platform=linux/arm64,linux/amd64").InDir("examples/nodejs").InNs(ns.Name).RunOrFailOutput(t)
outputBytes := skaffold.Build("--quiet", "--platform=linux/arm64,linux/amd64", "--cache-artifacts=false", "--tag", uuid.New().String()).InDir("examples/nodejs").InNs(ns.Name).RunOrFailOutput(t)
// Parse the Build Output
buildArtifacts, err := flags.ParseBuildOutput(outputBytes)
failNowIfError(t, err)
Expand Down
122 changes: 122 additions & 0 deletions integration/remote_config_dependency_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/*
Copyright 2024 The Skaffold Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package integration

import (
"testing"

"github.com/GoogleContainerTools/skaffold/v2/integration/skaffold"
"github.com/GoogleContainerTools/skaffold/v2/testutil"
)

func TestRenderWithGCBRepositoryRemoteDependency(t *testing.T) {
tests := []struct {
description string
configFile string
shouldErr bool
expectedOutput string
expectedErrMsg string
}{
{
description: "GCB repository remote dependency with private git repo",
configFile: `apiVersion: skaffold/v4beta10
kind: Config
requires:
- googleCloudBuildRepoV2:
projectID: k8s-skaffold
region: us-central1
connection: github-connection-e2e-tests
repo: skaffold-getting-started
`,
expectedOutput: `apiVersion: v1
kind: Pod
metadata:
name: getting-started
spec:
containers:
- image: skaffold-example:fixed
name: getting-started
`,
},
{
description: "GCB repository remote dependency with private git repo, pointing to an specific branch",
configFile: `apiVersion: skaffold/v4beta10
kind: Config
requires:
- googleCloudBuildRepoV2:
projectID: k8s-skaffold
region: us-central1
connection: github-connection-e2e-tests
repo: skaffold-getting-started
ref: feature-branch
`,
expectedOutput: `apiVersion: apps/v1
kind: Deployment
metadata:
name: my-deployment
labels:
app: my-deployment
spec:
replicas: 1
selector:
matchLabels:
app: my-deployment
template:
metadata:
labels:
app: my-deployment
spec:
containers:
- name: getting-started
image: skaffold-example-deployment:fixed
`,
},
{
description: "GCB repository remote dependency with private git repo fails, bad configuration",
configFile: `apiVersion: skaffold/v4beta10
kind: Config
requires:
- googleCloudBuildRepoV2:
projectID: bad-repo
region: us-central1
connection: github-connection-e2e-tests
repo: skaffold-getting-started
ref: feature-branch
`,
shouldErr: true,
expectedErrMsg: "getting GCB repo info for skaffold-getting-started: failed to get remote URI for repository skaffold-getting-started",
},
}

for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
MarkIntegrationTest(t.T, NeedsGcp)
tmpDir := t.NewTempDir()
tmpDir.Write("skaffold.yaml", test.configFile)
args := []string{"--remote-cache-dir", tmpDir.Root(), "--tag", "fixed", "--default-repo=", "--digest-source", "tag"}
output, err := skaffold.Render(args...).InDir(tmpDir.Root()).RunWithCombinedOutput(t.T)

t.CheckError(test.shouldErr, err)

if !test.shouldErr {
t.CheckDeepEqual(test.expectedOutput, string(output), testutil.YamlObj(t.T))
} else {
t.CheckContains(test.expectedErrMsg, string(output))
}
})
}
}
8 changes: 0 additions & 8 deletions pkg/skaffold/build/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,6 @@ type ImageDetails struct {
ID string `yaml:"id,omitempty"`
}

func (d ImageDetails) HasID() bool {
return d.ID != ""
}

func (d ImageDetails) HasDigest() bool {
return d.Digest != ""
}

// ArtifactCache is a map of [artifact dependencies hash : ImageDetails]
type ArtifactCache map[string]ImageDetails

Expand Down
71 changes: 28 additions & 43 deletions pkg/skaffold/build/cache/lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,29 +68,30 @@ func (c *cache) lookup(ctx context.Context, a *latest.Artifact, tag string, plat
return failed{err: fmt.Errorf("getting hash for artifact %q: %s", a.ImageName, err)}
}

c.cacheMutex.RLock()
entry, cacheHit := c.artifactCache[hash]
c.cacheMutex.RUnlock()

pls := platforms.GetPlatforms(a.ImageName)
// TODO (gaghosh): allow `tryImport` when the Docker daemon starts supporting multiarch images
// See https://github.com/docker/buildx/issues/1220#issuecomment-1189996403
if !cacheHit && !pls.IsMultiPlatform() {
var pl v1.Platform
if len(pls.Platforms) == 1 {
pl = util.ConvertToV1Platform(pls.Platforms[0])
}
if entry, err = c.tryImport(ctx, a, tag, hash, pl); err != nil {
log.Entry(ctx).Debugf("Could not import artifact from Docker, building instead (%s)", err)
return needsBuilding{hash: hash}
}
}

if isLocal, err := c.isLocalImage(a.ImageName); err != nil {
return failed{err}
} else if isLocal {
c.cacheMutex.RLock()
entry, cacheHit := c.artifactCache[hash]
c.cacheMutex.RUnlock()

// TODO (gaghosh): allow `tryImport` when the Docker daemon starts supporting multiarch images
// See https://github.com/docker/buildx/issues/1220#issuecomment-1189996403
if !cacheHit && !pls.IsMultiPlatform() {
var pl v1.Platform
if len(pls.Platforms) == 1 {
pl = util.ConvertToV1Platform(pls.Platforms[0])
}
if entry, err = c.tryImport(ctx, a, tag, hash, pl); err != nil {
log.Entry(ctx).Debugf("Could not import artifact from Docker, building instead (%s)", err)
return needsBuilding{hash: hash}
}
}
return c.lookupLocal(ctx, hash, tag, entry)
}
return c.lookupRemote(ctx, hash, tag, pls.Platforms)
return c.lookupRemote(ctx, hash, tag, pls.Platforms, entry)
}

func (c *cache) lookupLocal(ctx context.Context, hash, tag string, entry ImageDetails) cacheDetails {
Expand Down Expand Up @@ -118,41 +119,25 @@ func (c *cache) lookupLocal(ctx context.Context, hash, tag string, entry ImageDe
return needsBuilding{hash: hash}
}

func (c *cache) lookupRemote(ctx context.Context, hash, tag string, platforms []specs.Platform) cacheDetails {
c.cacheMutex.RLock()
cachedEntry, cacheHit := c.artifactCache[hash]
c.cacheMutex.RUnlock()

func (c *cache) lookupRemote(ctx context.Context, hash, tag string, platforms []specs.Platform, entry ImageDetails) cacheDetails {
if remoteDigest, err := docker.RemoteDigest(tag, c.cfg, nil); err == nil {
if cacheHit && remoteDigest == cachedEntry.Digest {
log.Entry(ctx).Debugf("Found %s remote with the same digest", tag)
// Image exists remotely with the same tag and digest
if remoteDigest == entry.Digest {
return found{hash: hash}
}

if !cacheHit {
log.Entry(ctx).Debugf("Added digest for %s to cache entry", tag)
cachedEntry.Digest = remoteDigest
c.cacheMutex.Lock()
c.artifactCache[hash] = cachedEntry
c.cacheMutex.Unlock()
}
}

if cachedEntry.HasDigest() {
// Image exists remotely with a different tag
fqn := tag + "@" + cachedEntry.Digest // Actual tag will be ignored but we need the registry and the digest part of it.
log.Entry(ctx).Debugf("Looking up %s tag with the full fqn %s", tag, cachedEntry.Digest)
if remoteDigest, err := docker.RemoteDigest(fqn, c.cfg, nil); err == nil {
log.Entry(ctx).Debugf("Found %s with the full fqn", tag)
if remoteDigest == cachedEntry.Digest {
return needsRemoteTagging{hash: hash, tag: tag, digest: cachedEntry.Digest, platforms: platforms}
}
// Image exists remotely with a different tag
fqn := tag + "@" + entry.Digest // Actual tag will be ignored but we need the registry and the digest part of it.
if remoteDigest, err := docker.RemoteDigest(fqn, c.cfg, nil); err == nil {
if remoteDigest == entry.Digest {
return needsRemoteTagging{hash: hash, tag: tag, digest: entry.Digest, platforms: platforms}
}
}

// Image exists locally
if cachedEntry.HasID() && c.client != nil && c.client.ImageExists(ctx, cachedEntry.ID) {
return needsPushing{hash: hash, tag: tag, imageID: cachedEntry.ID}
if entry.ID != "" && c.client != nil && c.client.ImageExists(ctx, entry.ID) {
return needsPushing{hash: hash, tag: tag, imageID: entry.ID}
}

return needsBuilding{hash: hash}
Expand Down

0 comments on commit 0137e1e

Please sign in to comment.