Skip to content

tests: avoid deprecated kubectl run in secrets scenario #3847

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.

Sign up for GitHub

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

Jump to bottom
Merged
openshift-merge-robot merged 2 commits into from
Jan 2, 2020
Merged

tests: avoid deprecated kubectl run in secrets scenario #3847

openshift-merge-robot merged 2 commits into from
Jan 2, 2020

Conversation

@spadgett
Copy link
Member

@spadgett spadgett commented Jan 2, 2020 •
edited by rhamilto
Loading

and fix bug in monitoring scenario.

@spadgett spadgett added the kind/test-flake Categorizes issue or PR as related to test flakes. label Jan 2, 2020
@spadgett spadgett added this to the v4.4 milestone Jan 2, 2020
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 2, 2020
@spadgett
Copy link
Member Author

spadgett commented Jan 2, 2020

/test analyze

@spadgett
Copy link
Member Author

spadgett commented Jan 2, 2020

/assign @rhamilto @benjaminapetersen

This should unblock CI

rhamilto
rhamilto approved these changes Jan 2, 2020
View reviewed changes
Copy link
Member

@rhamilto rhamilto 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 Jan 2, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 2, 2020
@spadgett
Copy link
Member Author

spadgett commented Jan 2, 2020

I added some clean up logic since we weren't deleting the resources. @rhamilto another look?

@rhamilto
Copy link
Member

rhamilto commented Jan 2, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 2, 2020
@andrewballantyne
Copy link
Contributor

andrewballantyne commented Jan 2, 2020

ci/prow/analyze failed because of output size?

FAILURE: Main vendor bundle is larger than the 2 MiB limit.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 2, 2020

/retest

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

benjaminapetersen
benjaminapetersen approved these changes Jan 2, 2020
View reviewed changes
Copy link
Contributor

@benjaminapetersen benjaminapetersen left a comment

Choose a reason for hiding this comment

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

/lgtm

@benjaminapetersen
Copy link
Contributor

benjaminapetersen commented Jan 2, 2020

hmm. that analyze job has been quirky as well.

@andrewballantyne
Copy link
Contributor

andrewballantyne commented Jan 2, 2020

Seems like a webpack issue? How'd this ever get in if there is a dependency bloating issue?

Main vendor bundle size: 3.85 MiB
FAILURE: Main vendor bundle is larger than the 2 MiB limit.
If you haven't added a new dependency, an import might have accidentally pulled an existing dependency into the main vendor bundle.
If adding a large dependency, consider lazy loading the component with AsyncComponent.

@spadgett
Copy link
Member Author

spadgett commented Jan 2, 2020

@andrewballantyne I've seen yarn run analyze give different results different times you run it with no other code changes :(

Seems to be from the PF icons:

https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_console/3847/pull-ci-openshift-console-master-analyze/2785/artifacts/analyze/report.html

@spadgett
Copy link
Member Author

spadgett commented Jan 2, 2020

/test analyze

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 2, 2020

/retest

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

@andrewballantyne
Copy link
Contributor

andrewballantyne commented Jan 2, 2020

@spadgett perhaps I am misunderstanding the build process here... but if it sometimes packages a larger set of icons, wouldn't this run the risk of an actual release being bloated? Or do we run a similar / the same test against the release output?

I am also confused how packaging could produce a non-deterministic result.

@spadgett
Copy link
Member Author

spadgett commented Jan 2, 2020

Yeah, it's alarming. I'm worried about this as well. We need to get to the bottom of it.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 2, 2020

/retest

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

@spadgett
Copy link
Member Author

spadgett commented Jan 2, 2020

/hold

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

openshift-ci-robot commented Jan 2, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benjaminapetersen, rhamilto, spadgett

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

The pull request process is described here

Details 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-merge-robot openshift-merge-robot merged commit b2b0936 into openshift:master Jan 2, 2020
This was referenced Jan 2, 2020
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
This was referenced Jan 2, 2020
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@spadgett spadgett deleted the secret-scenario branch January 3, 2020 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rhamilto rhamilto rhamilto approved these changes

@jhadvig jhadvig Awaiting requested review from jhadvig

+2 more reviewers

@andrewballantyne andrewballantyne andrewballantyne left review comments

@benjaminapetersen benjaminapetersen benjaminapetersen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@benjaminapetersen benjaminapetersen

@rhamilto rhamilto

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/test-flake Categorizes issue or PR as related to test flakes. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

v4.4

Development

Successfully merging this pull request may close these issues.

7 participants

@spadgett @rhamilto @andrewballantyne @openshift-bot @benjaminapetersen @openshift-ci-robot @openshift-merge-robot