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

Bug 1820687: NS SCC annotations exist, else forbidden: unable to validate... #24828

Merged

Conversation

gabemontero
Copy link
Contributor

@gabemontero gabemontero commented Apr 3, 2020

Stems from debug in #24703

@openshift/openshift-team-developer-experience FYI

/assign @adambkaplan
(appears is in a sufficient OWNERS file)

@openshift-ci-robot
Copy link

@gabemontero: This pull request references Bugzilla bug 1820687, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.5.0) matches configured target release for branch (4.5.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1820687: NS SCC annotations exist, else forbidden: unable to validate...

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Apr 3, 2020
@openshift-ci-robot
Copy link

@gabemontero: This pull request references Bugzilla bug 1820687, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.5.0) matches configured target release for branch (4.5.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1820687: NS SCC annotations exist, else forbidden: unable to validate...

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@gabemontero
Copy link
Contributor Author

/test e2e-gcp-image-ecosystem

@gabemontero
Copy link
Contributor Author

/hold

given the flake level of this @adambkaplan I am going to run a series of image ecos here to try and validate

but please feel free to approve/lgtm in the interim

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 3, 2020
@gabemontero
Copy link
Contributor Author

initial image eco run good, along with a couple of other e2e suites

/test unit
/test images
/test e2e-cmd

@gabemontero
Copy link
Contributor Author

Hah ... I didn't think the k8s 1.18 rebase landed anywhere in origin, but cmd/images are failing with

++ Building go targets for linux/amd64: cmd/openshift-tests
# github.com/openshift/origin/test/extended/util
test/extended/util/framework.go:949:32: not enough arguments in call to c.Projects().Get
	have (string, "github.com/openshift/origin/vendor/k8s.io/apimachinery/pkg/apis/meta/v1".GetOptions)
	want (context.Context, string, "github.com/openshift/origin/vendor/k8s.io/apimachinery/pkg/apis/meta/v1".GetOptions)
[ERROR] hack/lib/build/binaries.sh:224: `GOOS=${platform%/*} GOARCH=${platform##*/} go install -tags "${OS_GOFLAGS_TAGS-} ${!platform_gotags_envvar:-}" -ldflags="${local_ldflags}" "${goflags[@]:+${goflags[@]}}" -gcflags "${gogcflags}" "${nonstatics[@]}"` exited with status 2.
[ERROR] hack/lib/build/binaries.sh:138: `local -a binaries=("$@")` exited with status 2.
[ERROR] hack/build-go.sh exited with code 2 after 00h 01m 28s

I'll rebase my branch ... perhaps something just merged.

@gabemontero
Copy link
Contributor Author

sure enough ... that was it ... rebase will be pushed momentarily

@gabemontero
Copy link
Contributor Author

/retest

@gabemontero
Copy link
Contributor Author

/test e2e-cmd

@gabemontero
Copy link
Contributor Author

/test e2e-gcp-image-ecosystem
/test e2e-aws-image-ecosystem

@gabemontero
Copy link
Contributor Author

have all green tests .... re-runnig image eco a bunch of times

@gabemontero
Copy link
Contributor Author

/test e2e-gcp-image-ecosystem

@gabemontero
Copy link
Contributor Author

So a promising sign, in the last run, there were a series of
namespace_scc_allocation_controller.go:336] error syncing namespace, it will be retried: Operation cannot be fulfilled on namespaces "e2e-test-s2i-usage-ng2cm": the object has been modified; please apply your changes to the latest version and try again

type errors in the cluster-policy-controller at the same time the SCL enabled Pod launch tests were going on, and we got no forbidden errors.

I'm also seeing some test duration times that are a minute or two longer (even across the same language stack) that could indicate a wait/retry cycle waiting on the SCC annotations.

@gabemontero
Copy link
Contributor Author

gabemontero commented Apr 6, 2020

Similar finding to #24828 (comment) in last run.
And the tests that ran a minute or two longer were not the same ones as the prior run.

Again encouraging.

/test e2e-gcp-image-ecosystem

@adambkaplan
Copy link
Contributor

@gabemontero to me this implies there is a product bug in the cluster-policy-controller where it is conflicting too often, and thus not able to keep up with namespace creation. Is there a separate BZ for this?

@gabemontero
Copy link
Contributor Author

@gabemontero to me this implies there is a product bug in the cluster-policy-controller where it is conflicting too often, and thus not able to keep up with namespace creation. Is there a separate BZ for this?

Don't necessarily share that opinion @adambkaplan and so no I've not opened a separate BZ at this time. I debated some changes to cluster-policy-controller in my original debug PR, but in the end I don't think they would amount to much.

@gabemontero
Copy link
Contributor Author

/test e2e-gcp-image-ecosystem

2 similar comments
@gabemontero
Copy link
Contributor Author

/test e2e-gcp-image-ecosystem

@gabemontero
Copy link
Contributor Author

/test e2e-gcp-image-ecosystem

test/extended/util/framework.go Outdated Show resolved Hide resolved
test/extended/util/framework.go Outdated Show resolved Hide resolved
@gabemontero
Copy link
Contributor Author

update for #24828 (comment) and #24828 (comment) pushed @adambkaplan

Copy link
Contributor

@adambkaplan adambkaplan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gabemontero comment nit, otherwise looks good.

test/extended/util/framework.go Outdated Show resolved Hide resolved
@gabemontero
Copy link
Contributor Author

suggestion accepted @adambkaplan

commits squashed

/hold cancel

tag at your convenience - thanks !!

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 7, 2020
@gabemontero
Copy link
Contributor Author

/retest

1 similar comment
@gabemontero
Copy link
Contributor Author

/retest

Copy link
Contributor

@adambkaplan adambkaplan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 13, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adambkaplan, gabemontero

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 13, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 4d0922f into openshift:master Apr 13, 2020
@openshift-ci-robot
Copy link

@gabemontero: Some pull requests linked via external trackers have merged: . The following pull requests linked via external trackers have not merged:

In response to this:

Bug 1820687: NS SCC annotations exist, else forbidden: unable to validate...

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@soltysh
Copy link
Member

soltysh commented Jun 18, 2020

/cherry-pick release-4.4

@openshift-cherrypick-robot

@soltysh: #24828 failed to apply on top of branch "release-4.4":

error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	test/extended/util/client.go
M	test/extended/util/framework.go
Falling back to patching base and 3-way merge...
Auto-merging test/extended/util/framework.go
CONFLICT (content): Merge conflict in test/extended/util/framework.go
Auto-merging test/extended/util/client.go
CONFLICT (content): Merge conflict in test/extended/util/client.go
Patch failed at 0001 NS SCC annotations exist, else forbidden: unable to validate...

In response to this:

/cherry-pick release-4.4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants