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

bump to kubernetes-1.17.0-beta.2 #44

Merged
merged 19 commits into from Nov 20, 2019

Conversation

mfojtik
Copy link
Member

@mfojtik mfojtik commented Nov 15, 2019

This moves openshift-apiserver to kubernetes 1.17.0-beta.2 which include dynamic certificate reloading and other fixes in aggregated apiservers.

The apiserver-library-go was also bumped to 1.17.0-beta.2: https://github.com/openshift/apiserver-library-go/tree/openshift-apiserver-4.3-kubernetes-1.17.0-beta.2 (surprisingly tolerating library-go still being on 1.16.2).

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 15, 2019
@mfojtik
Copy link
Member Author

mfojtik commented Nov 15, 2019

@deads2k as promised

@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 15, 2019
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 15, 2019
@mfojtik
Copy link
Member Author

mfojtik commented Nov 15, 2019

/hold

we need kube tag to move forward

@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 Nov 15, 2019
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 19, 2019
@mfojtik mfojtik changed the title WIP: bump to kubernetes-1.17.0-beta.1 + dynamic reload patches WIP: bump to kubernetes-1.17.0-beta.2 Nov 19, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 19, 2019
@mfojtik
Copy link
Member Author

mfojtik commented Nov 19, 2019

@sttts the openapi generation fail with:

E1119 14:01:39.582048   59789 openapi.go:304] Error when generating: managementState, ManagementState github.com/openshift/api/operator/v1.ManagementState
2019/11/19 14:01:39 OpenAPI code generation error: Failed executing generator: some packages had errors:
Tags in comment and struct should match for member (ManagementState) of (github.com/openshift/api/samples/v1.ConfigStatus)

/cc @damemi

@@ -15,6 +15,10 @@ func TestLogOptionsDrift(t *testing.T) {
// Verify name
name := popts.Field(i).Name
doptsField, found := dopts.FieldByName(name)
// TODO: Add this option to deployment config log options
Copy link
Contributor

Choose a reason for hiding this comment

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

followup with a BZ seems fine to me

Copy link
Contributor

Choose a reason for hiding this comment

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

followup with a BZ seems fine to me

Yeah, BZ for 4.4. it's not urgent.

@deads2k
Copy link
Contributor

deads2k commented Nov 19, 2019

fairly minor comments. The biggest one is the question of fuzzing tests.

@mfojtik
Copy link
Member Author

mfojtik commented Nov 19, 2019

Follow up list:

  1. Make a BZ for new log options field
  2. Make a BZ doe @openshift/openshift-team-developer-experience for disabled unit test

@mfojtik
Copy link
Member Author

mfojtik commented Nov 20, 2019

@deads2k whoah e2e passed :)

@mfojtik
Copy link
Member Author

mfojtik commented Nov 20, 2019

/retest

@mfojtik mfojtik force-pushed the bump-kube-1.17 branch 2 times, most recently from 2868498 to d225f3d Compare November 20, 2019 11:51
@mfojtik mfojtik changed the title WIP: bump to kubernetes-1.17.0-beta.2 bump to kubernetes-1.17.0-beta.2 Nov 20, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 20, 2019
@mfojtik
Copy link
Member Author

mfojtik commented Nov 20, 2019

/hold cancel

@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 Nov 20, 2019
@mfojtik
Copy link
Member Author

mfojtik commented Nov 20, 2019

@deads2k @sttts @adambkaplan @bparees this is ready for review

@@ -174,16 +174,6 @@ func RegisterConversions(s *runtime.Scheme) error {
}); err != nil {
return err
}
if err := s.AddGeneratedConversionFunc((*v1.DeploymentTriggerImageChangeParams)(nil), (*apps.DeploymentTriggerImageChangeParams)(nil), func(a, b interface{}, scope conversion.Scope) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

reminder for future me. These were actually registered twice before. The fuzz tests appear to be working, so we're good.

@deads2k
Copy link
Contributor

deads2k commented Nov 20, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 20, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, mfojtik

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-merge-robot openshift-merge-robot merged commit 2e7b3f6 into openshift:master Nov 20, 2019
deads2k added a commit to deads2k/openshift-apiserver that referenced this pull request Dec 13, 2019
deads2k added a commit to deads2k/openshift-apiserver that referenced this pull request Dec 13, 2019
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. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants