Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check-schema-changes has confusing error when released schema is modified #4168

Closed
prary opened this issue May 13, 2020 · 2 comments · Fixed by #4355
Closed

check-schema-changes has confusing error when released schema is modified #4168

prary opened this issue May 13, 2020 · 2 comments · Fixed by #4355
Assignees
Labels
kind/bug Something isn't working meta/development priority/p3 agreed that this would be good to have, but no one is available at the moment.

Comments

@prary
Copy link
Contributor

prary commented May 13, 2020

Was fixing my stale PR https://github.com/GoogleContainerTools/skaffold/pull/3346 which earlier failed on make checks for some import disorder. Merged latest master code and ran make checks which again failed on hack/check-schema-changes.sh. I am bit confused how master builds are passing. Tried with fresh cloned code on master branch but failing on the hack/check-schema-changes.sh

Contributing guideline suggest to run ./hack/new_version.sh but I am not making changing schema.

Expected behavior

make checks should run successfully.

Actual behavior

Running validation scripts...
RUN hack/check-schema-changes.sh
INFO[0000] structural changes in latest config, checking on Github if latest is released... 
INFO[0000] Current Skaffold version: v2beta3            
v2beta3 is released, it should NOT be changed!
ERRO[0000] --------
Structural change detected in a released config: pkg/skaffold/schema/latest/config.go
Please create a new PR first with a new version.
You can use 'hack/new_version.sh' to generate the new config version.
If you are running this locally, make sure you have the origin/master branch up to date!
Admin rights are required to merge this PR!
-------- 
diff --git a/pkg/skaffold/schema/latest/config.go b/pkg/skaffold/schema/latest/config.go
index fd08129f4..a1cdee094 100644
--- a/pkg/skaffold/schema/latest/config.go
+++ b/pkg/skaffold/schema/latest/config.go
@@ -22,7 +22,7 @@ import (
        "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
 )
 
-// This config version is not yet released, it is SAFE TO MODIFY the structs in this file.
+// !!! WARNING !!! This config version is already released, please DO NOT MODIFY the structs in this file.
 const Version string = "skaffold/v2beta3"
 
 // NewSkaffoldConfig creates a SkaffoldConfig
@@ -317,6 +317,15 @@ type ClusterDetails struct {
        // DockerConfig describes how to mount the local Docker configuration into a pod.
        DockerConfig *DockerConfig `yaml:"dockerConfig,omitempty"`
 
+       // ServiceAccountName describes the Kubernetes service account to use for the pod.
+       // Defaults to 'default'.
+       ServiceAccountName string `yaml:"serviceAccount,omitempty"`
+
+       // RunAsUser defines the UID to request for running the container.
+       // If omitted, no SeurityContext will be specified for the pod and will therefore be inherited
+       // from the service account.
+       RunAsUser *int64 `yaml:"runAsUser,omitempty"`
+
        // Resources define the resource requirements for the kaniko pod.
        Resources *ResourceRequirements `yaml:"resources,omitempty"`

Output of ./hack/new_version.sh

ok      github.com/GoogleContainerTools/skaffold/cmd/skaffold/app       0.048s
FAIL    github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd   0.089s
ok      github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/config    (cached)
ok      github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/schema    0.028s
ok      github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/flags (cached)
FAIL    github.com/GoogleContainerTools/skaffold/hack/man       0.059s
ok      github.com/GoogleContainerTools/skaffold/hack/schemas   0.892s
ok      github.com/GoogleContainerTools/skaffold/hack/versions/pkg/diff 0.034s
ok      github.com/GoogleContainerTools/skaffold/hack/versions/pkg/schema       (cached)
ok      github.com/GoogleContainerTools/skaffold/pkg/webhook/labels     (cached)

=== Failed Tests ===
/pkg/skaffold/schema/TestParseExamples
--- FAIL: TestParseExamples (0.00s)
/pkg/skaffold/schema/TestParseExamples/bazel
    TestParseExamples/bazel: samples_test.go:79: unexpected error: skaffold/v2bet3 is an invalid api version
    --- FAIL: TestParseExamples/bazel (0.00s)
panic: interface conversion: util.VersionedConfig is nil, not *latest.SkaffoldConfig [recovered]
        panic: interface conversion: util.VersionedConfig is nil, not *latest.SkaffoldConfig

Information

  • Skaffold version: NA
  • Operating system: linux
  • Contents of skaffold.yaml:
  • Go Version: go version go1.13.6 linux/amd64
@dgageot
Copy link
Contributor

dgageot commented May 14, 2020

@prary This fails because your PR makes changes to a schema that was already released. You cannot change anything in pkg/skaffold/schema/latest/config.go b/pkg/skaffold/schema/latest/config.go. It requires to create a new schema, which @tejal29 just did.

@tstromberg tstromberg changed the title Build Pipeline failing Build Pipeline failing (editing a schema that already exists) May 18, 2020
@tstromberg
Copy link
Contributor

I think we can make the error message here clearer.

@tstromberg tstromberg changed the title Build Pipeline failing (editing a schema that already exists) check-schema-changes has confusing error when released schema is modified May 18, 2020
@tstromberg tstromberg added kind/bug Something isn't working priority/p3 agreed that this would be good to have, but no one is available at the moment. labels May 18, 2020
dgageot added a commit to dgageot/skaffold that referenced this issue Jun 19, 2020
@dgageot dgageot self-assigned this Jun 19, 2020
dgageot added a commit to dgageot/skaffold that referenced this issue Jun 19, 2020
dgageot added a commit to dgageot/skaffold that referenced this issue Jun 19, 2020
dgageot added a commit to dgageot/skaffold that referenced this issue Jun 19, 2020
dgageot added a commit to dgageot/skaffold that referenced this issue Jun 19, 2020
dgageot added a commit that referenced this issue Jun 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working meta/development priority/p3 agreed that this would be good to have, but no one is available at the moment.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants