Skip to content

[WIP] Ignore this, testing kube 1.28 bump#3976

Closed
jkyros wants to merge 8 commits into
openshift:masterfrom
jkyros:bump-kube-1.28
Closed

[WIP] Ignore this, testing kube 1.28 bump#3976
jkyros wants to merge 8 commits into
openshift:masterfrom
jkyros:bump-kube-1.28

Conversation

@jkyros
Copy link
Copy Markdown
Member

@jkyros jkyros commented Oct 13, 2023

Just checking to see if the newer API is what's causing us to fail, or if it's something in our API migration.

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 13, 2023
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Oct 13, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@jkyros
Copy link
Copy Markdown
Member Author

jkyros commented Oct 13, 2023

/payload-job periodic-ci-openshift-hypershift-release-4.15-periodics-e2e-aws-ovn-conformance

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Oct 13, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jkyros

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-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 13, 2023
@jkyros
Copy link
Copy Markdown
Member Author

jkyros commented Oct 13, 2023

hmm do I have to be out of draft before I can trigger a payload job? weird.

@jkyros jkyros marked this pull request as ready for review October 13, 2023 19:06
@jkyros
Copy link
Copy Markdown
Member Author

jkyros commented Oct 13, 2023

/payload-job periodic-ci-openshift-hypershift-release-4.15-periodics-e2e-aws-ovn-conformance

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Oct 13, 2023

@jkyros: trigger 1 job(s) for the /payload-(job|aggregate) command

  • periodic-ci-openshift-hypershift-release-4.15-periodics-e2e-aws-ovn-conformance

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/c04b4450-69fb-11ee-8c8a-1cc375c1d9e8-0

@jkyros
Copy link
Copy Markdown
Member Author

jkyros commented Oct 13, 2023

apparently yes? Or it has to happen after that first bot checkin/approval or something.

@jkyros
Copy link
Copy Markdown
Member Author

jkyros commented Oct 13, 2023

So if this fails, but #3958 passed, I will suspect something is rotten in the newer libraries.

If this one also passes, then I've probably broken it somehow with the API migration and I'll need to understand how.

jkyros and others added 7 commits October 13, 2023 18:29
We've been keeping the kubelet.yaml template in sync with the
openshift/api featuregates manually for quite awhile. A bunch of new
featuregates graduated, so we're going to do one more round, but we
do need to come back and do this the right way by reading the default
featuregates off the featureGateAccessor and injecing them into the
kubelet.yaml templates. The plumbing is already there, we just need to
use it.

I also added some additional verbosity to the skew test because I got
tired of having to hunt through the list for the missing gate :)
This updates our imports to read the API from openshift/api,
removes the generated and API packages (because client-go supersedes
them) and moves the api helper functions to their own package
"apihelpers" since they did not go with the API.

This also removes the "legacy" MCO api from the repo, so anyone using it
will either have to pin to an older version or migreate to using it from
openshift/api
There were a few API helper functions that were part of the API when it
lived here, but since they were only used here in the MCO, we didn't
migrate them to openshift/api, we just broke them out into a separate
apihelpers package.

Initially we thought we might be able to stuff them into
controller/common but with the way the test suite is setup, that results
in a dependency cycle, so a separate package was the way to go.
We merged some additional code since I went through and changed all the
imports and references, this just makes sure the new code we've added is
properly referencing the MCO API and its helpers.
We took these certificate observability fields out because they were the
wrong type and put in a storageversionmigration for them in 4.14, but
now that we're in 4.15, and we have a newly generated client-go for the
MCO that uses applyconfigurations/server-side apply, it matters that
we're not supplying the "whole object" every time -- "partially" applying
them like we were as a stopgap will now generate errors and fail.

This reintroduces those time fields to the MCO code so
certificate observability work again, and the "required field not
supplied" errors will cease.
@jkyros
Copy link
Copy Markdown
Member Author

jkyros commented Oct 13, 2023

alright, so the api + library bumps without the API changes work, so that rules that out. Let's try it with the API but without the CRD location changes?

@jkyros
Copy link
Copy Markdown
Member Author

jkyros commented Oct 13, 2023

/payload-job periodic-ci-openshift-hypershift-release-4.15-periodics-e2e-aws-ovn-conformance

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Oct 13, 2023

@jkyros: trigger 1 job(s) for the /payload-(job|aggregate) command

  • periodic-ci-openshift-hypershift-release-4.15-periodics-e2e-aws-ovn-conformance

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/852d6b50-6a23-11ee-8240-67fb7925d26f-0

@jkyros
Copy link
Copy Markdown
Member Author

jkyros commented Oct 14, 2023

hah, okay, the API is fine, it looks like it's moving the CRDs that breaks it? Maybe something to do with https://github.com/openshift/hypershift/blob/15c5cddfb44ca2636deca17fd6c3367336aff287/control-plane-operator/controllers/hostedcontrolplane/cvo/reconcile.go#L49 ? Since hypershift cares about the manifests? (Or it's flaky and it just happens to be pointing my way this time. )

@jkyros
Copy link
Copy Markdown
Member Author

jkyros commented Oct 17, 2023

/payload-job periodic-ci-openshift-hypershift-release-4.15-periodics-e2e-aws-ovn-conformance

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Oct 17, 2023

@jkyros: trigger 1 job(s) for the /payload-(job|aggregate) command

  • periodic-ci-openshift-hypershift-release-4.15-periodics-e2e-aws-ovn-conformance

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/87c0c250-6d42-11ee-9dd3-d24a28d6aa97-0

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Oct 18, 2023

@jkyros: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/okd-scos-e2e-aws-ovn 47cccf2 link false /test okd-scos-e2e-aws-ovn

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@jkyros jkyros closed this Oct 20, 2023
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. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant