Skip to content

Comments

Add baremetal ovn job using local gateway mode#18539

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
jluhrsen:ovn-local-gw-bm
Jul 7, 2021
Merged

Add baremetal ovn job using local gateway mode#18539
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
jluhrsen:ovn-local-gw-bm

Conversation

@jluhrsen
Copy link
Contributor

@jluhrsen jluhrsen commented May 12, 2021

Signed-off-by: Jamo Luhrsen jluhrsen@gmail.com

@openshift-ci openshift-ci bot requested review from smarterclayton and vrutkovs May 12, 2021 21:43
@jluhrsen jluhrsen mentioned this pull request May 12, 2021
Merged
@jluhrsen
Copy link
Contributor Author

jluhrsen commented May 12, 2021

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 12, 2021
@jluhrsen
Copy link
Contributor Author

jluhrsen commented May 12, 2021

this PR is meaningless until this one in dev-scripts is merged

@jluhrsen
Copy link
Contributor Author

jluhrsen commented May 13, 2021

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 13, 2021
@stbenjam
Copy link
Member

stbenjam commented May 14, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 14, 2021
@jluhrsen
Copy link
Contributor Author

jluhrsen commented May 17, 2021

metal-ipi jobs do not work with rehearsals, so we have to merge this without seeing that. it's a new job though and I can monitor it in case something funky is broken.

@jluhrsen
Copy link
Contributor Author

jluhrsen commented May 17, 2021

@vrutkovs , can we get this in please?

vrutkovs
vrutkovs approved these changes May 18, 2021
View reviewed changes
Copy link
Contributor

@vrutkovs vrutkovs left a comment

Choose a reason for hiding this comment

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

/approve

@vrutkovs
Copy link
Contributor

vrutkovs commented May 18, 2021

Seems you need someone else to approve that.
/cc wking smarterclayton

metal-ipi jobs do not work with rehearsals

Wait, why?

@openshift-ci openshift-ci bot requested a review from wking May 18, 2021 07:30
@jluhrsen
Copy link
Contributor Author

jluhrsen commented May 18, 2021

Seems you need someone else to approve that.
/cc wking smarterclayton

metal-ipi jobs do not work with rehearsals

Wait, why?

because the config is too big and we hit some OS limit. I can't find one PR that I was more verbose about it,
but here is one where I was able to remove some steps to reduce the config and let the rehearse job run
the steps that I was changing to show the PR was going to work. It points to this slack discussion as well.

@vrutkovs
Copy link
Contributor

vrutkovs commented May 18, 2021

If it won't pass rehearsals now, it probably would be permafailing in periodics

/retest

@jluhrsen
Copy link
Contributor Author

jluhrsen commented May 18, 2021

If it won't pass rehearsals now, it probably would be permafailing in periodics

/retest

no, the periodics are fine, like this, but here is what a rehearsal looks like.

@stbenjam
Copy link
Member

stbenjam commented May 21, 2021

If it won't pass rehearsals now, it probably would be permafailing in periodics

/retest

It's a problem with rehearsals only, the entire job contents including all step metadata and scripts are stored in an environment variable and we hit the max limit (something like ~128kB).

We're working on reducing the size.

@stbenjam
Copy link
Member

stbenjam commented May 22, 2021

/test pj-rehearse

It might rehearse now.

@jluhrsen
Copy link
Contributor Author

jluhrsen commented May 22, 2021

/test pj-rehearse

it did rehearse now. Thanks @stbenjam. how did you get it working?

@stbenjam
Copy link
Member

stbenjam commented May 22, 2021

#17445 and #18583 made our jobs a lot smaller, we removed ~150 test skips. We've been slowly working our way through the conformance tests to make them work on IPv6/disconnected/baremetal, that was the latest batch.

The one test failure you see will be fixed by openshift-metal3/dev-scripts#1242 and the MCO PR it links to.

@jluhrsen
Copy link
Contributor Author

jluhrsen commented May 24, 2021

#17445 and #18583 made our jobs a lot smaller, we removed ~150 test skips. We've been slowly working our way through the conformance tests to make them work on IPv6/disconnected/baremetal, that was the latest batch.

The one test failure you see will be fixed by openshift-metal3/dev-scripts#1242 and the MCO PR it links to.

good stuff. thanks @stbenjam

@vrutkovs , can we get this approved please?

@vrutkovs
Copy link
Contributor

vrutkovs commented May 25, 2021

[sig-arch][Early] Managed cluster should start all core operators [Skipped:Disconnected] [Suite:openshift/conformance/parallel] fails:

fail [github.com/onsi/ginkgo@v4.7.0-origin.0+incompatible/internal/leafnodes/runner.go:113]: May 22 18:54:35.651: Some cluster operators are not ready: kube-apiserver (Upgradeable=False FeatureGates_RestrictedFeatureGates_IPv6DualStackNoUpgrade: FeatureGatesUpgradeable: "IPv6DualStackNoUpgrade" does not allow updates)

Sounds like we need a test skip / option for openshift-tests to test failure. @jluhrsen please make sure you file a bug for that.

Other than that it looks good,
/approve

@jluhrsen
Copy link
Contributor Author

jluhrsen commented May 26, 2021

/assign @joelsmith

@jluhrsen
Copy link
Contributor Author

jluhrsen commented Jun 4, 2021

/retest

1 similar comment
@jluhrsen
Copy link
Contributor Author

jluhrsen commented Jun 4, 2021

/retest

@stbenjam
Copy link
Member

stbenjam commented Jun 4, 2021

Failure is just because openshift/machine-config-operator#2580 hasn't landed yet, this needs a rebase but otherwise should be good to go

@openshift-ci openshift-ci bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed lgtm Indicates that a PR is ready to be merged. labels Jun 4, 2021
stbenjam
stbenjam reviewed Jun 7, 2021
View reviewed changes
ci-operator/jobs/openshift/release/openshift-release-master-periodics.yaml Outdated
Comment on lines 19297 to 19306
Copy link
Member

@stbenjam stbenjam Jun 7, 2021

Choose a reason for hiding this comment

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

Were these intentional changes?

Copy link
Contributor Author

@jluhrsen jluhrsen Jun 22, 2021

Choose a reason for hiding this comment

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

yes, this was an accident somehow. New force push just now to get it right and rebase on latest master

vrutkovs
vrutkovs approved these changes Jun 28, 2021
View reviewed changes
Copy link
Contributor

@vrutkovs vrutkovs 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 openshift-ci bot added lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 28, 2021
@openshift-ci openshift-ci bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 28, 2021
@jluhrsen
Copy link
Contributor Author

jluhrsen commented Jun 29, 2021

@vrutkovs , @stbenjam, this seems fine now. the only tests failing are soon to be skipped.

do we need to /override the failing job?

@vrutkovs
Copy link
Contributor

vrutkovs commented Jun 29, 2021

LGTM, lets add the same test for 4.9 nightlies too?

@jluhrsen
Add baremetal ovn job using local gateway mode
191f101 
Signed-off-by: Jamo Luhrsen <jluhrsen@gmail.com>
@jluhrsen
Copy link
Contributor Author

jluhrsen commented Jul 2, 2021

LGTM, lets add the same test for 4.9 nightlies too?

yep. just updated to do this.

@jluhrsen
Copy link
Contributor Author

jluhrsen commented Jul 2, 2021

/retest

2 similar comments
@jluhrsen
Copy link
Contributor Author

jluhrsen commented Jul 3, 2021

/retest

@jluhrsen
Copy link
Contributor Author

jluhrsen commented Jul 3, 2021

/retest

@stbenjam
Copy link
Member

stbenjam commented Jul 6, 2021

/approve

@jluhrsen
Copy link
Contributor Author

jluhrsen commented Jul 6, 2021

/approve

I think we need another /lgtm since we lost it when I added the same job for 4.9 on this PR

@vrutkovs
Copy link
Contributor

vrutkovs commented Jul 7, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 7, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 7, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jluhrsen, stbenjam, vrutkovs

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 9886ab4 into openshift:master Jul 7, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 7, 2021

@jluhrsen: Updated the following 2 configmaps:

  • ci-operator-master-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-release-master__nightly-4.8.yaml using file ci-operator/config/openshift/release/openshift-release-master__nightly-4.8.yaml
    • key openshift-release-master__nightly-4.9.yaml using file ci-operator/config/openshift/release/openshift-release-master__nightly-4.9.yaml
  • job-config-master configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-release-master-periodics.yaml using file ci-operator/jobs/openshift/release/openshift-release-master-periodics.yaml
Details

In response to this:

Signed-off-by: Jamo Luhrsen jluhrsen@gmail.com

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.

@jluhrsen jluhrsen deleted the ovn-local-gw-bm branch July 22, 2021 04:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smarterclayton smarterclayton Awaiting requested review from smarterclayton

@wking wking Awaiting requested review from wking

2 more reviewers

@stbenjam stbenjam stbenjam left review comments

@vrutkovs vrutkovs vrutkovs approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@vrutkovs vrutkovs

@joelsmith joelsmith

@stbenjam stbenjam

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jluhrsen @stbenjam @vrutkovs @joelsmith @openshift-merge-robot