Skip to content

Bump k8s 1.16.2 and openshift/* - #44

Merged
openshift-merge-robot merged 8 commits into
openshift:masterfrom
adambkaplan:v2-k8s-116-2
Nov 18, 2019
Merged

Bump k8s 1.16.2 and openshift/*#44
openshift-merge-robot merged 8 commits into
openshift:masterfrom
adambkaplan:v2-k8s-116-2

Conversation

@adambkaplan

@adambkaplan adambkaplan commented Oct 29, 2019

Copy link
Copy Markdown
Contributor
  1. Upgrade k8s to 1.16.2
  2. Bump openshift/* to pull in k8s 1.16 changes

@adambkaplan
go get k8s.io/*@kubernetes-1.16.2
a9c4e06 
- Includes go get k8s.io/kubernetes@v1.16.2
- go mod tidy
@adambkaplan
go mod vendor
5cac461
@adambkaplan
go get k8s.io/kubectl@kubernetes-1.16.2
e209faa
@adambkaplan
go mod tidy
a08834e
@adambkaplan
go mod vendor (2)
66e139f 
- Add k8s.io/kubectl@kubernetes-1.16.2
@adambkaplan
go get github.com/openshift/*@master
31ec729 
- Pull in k8s 1.16 changes for api, client-go, and library-go
@adambkaplan
go mod vendor
cea22bd
@adambkaplan

adambkaplan commented Oct 29, 2019

Copy link
Copy Markdown
Contributor Author

/assign @soltysh

@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 29, 2019
@adambkaplan

adambkaplan commented Oct 29, 2019

Copy link
Copy Markdown
Contributor Author

@gabemontero looks like the build controller start is racing with the internal registry hostname being published/updated

adambkaplan
adambkaplan commented Oct 31, 2019
View reviewed changes

@adambkaplan adambkaplan left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

/cc @tnozicka @ironcladlou @soltysh

Comment thread pkg/route/ingress/ingress_test.go
{
Name: "1-abcdef",
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","subdomain":"","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":8080}}}]`),
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":8080}}}]`),

@adambkaplan adambkaplan Oct 31, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ironcladlou ptal

Comment thread pkg/route/ingressip/service_ingressip_controller.go
ic.ipAllocator = ipallocator.NewAllocatorCIDRRange(ipNet, func(max int, rangeSpec string) allocator.Interface {
return allocator.NewAllocationMap(max, rangeSpec)
ic.ipAllocator, _ = ipallocator.NewAllocatorCIDRRange(ipNet, func(max int, rangeSpec string) (allocator.Interface, error) {
return allocator.NewAllocationMap(max, rangeSpec), nil

@adambkaplan adambkaplan Oct 31, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ironcladlou ptal

Comment thread pkg/apps/deployer/deployer_controller_test.go
p.Spec.PreemptionPolicy = nil
p.Spec.Overhead = nil
p.Spec.EphemeralContainers = nil
p.Spec.TopologySpreadConstraints = nil

@adambkaplan adambkaplan Oct 31, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tnozicka ptal

@tnozicka tnozicka Nov 4, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this change lgtm, would be nice to clean up the vendoring commits though

Comment thread pkg/authorization/defaultrolebindings/defaultrolebindings.go
defer klog.Infof("Shutting down DefaultRoleBindingController")

if !controller.WaitForCacheSync("DefaultRoleBindingController", stopCh, c.roleBindingSynced, c.namespaceSynced) {
if !cache.WaitForNamedCacheSync("DefaultRoleBindingController", stopCh, c.roleBindingSynced, c.namespaceSynced) {

@adambkaplan adambkaplan Oct 31, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@soltysh I believe this is the correct refactor

@adambkaplan

adambkaplan commented Nov 1, 2019

Copy link
Copy Markdown
Contributor Author

/retest

@ironcladlou

ironcladlou commented Nov 1, 2019

Copy link
Copy Markdown
Contributor

lgtm

@adambkaplan adambkaplan mentioned this pull request Nov 1, 2019
Closed
@adambkaplan

adambkaplan commented Nov 4, 2019

Copy link
Copy Markdown
Contributor Author

/retest

bump @soltysh

@adambkaplan

adambkaplan commented Nov 4, 2019

Copy link
Copy Markdown
Contributor Author

/hold

Given we're past feature freeze and the flakiness of the tests, this is too risky to make 4.3

@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 4, 2019
soltysh
soltysh approved these changes Nov 5, 2019
View reviewed changes

@soltysh soltysh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/lgtm

Feel free to unhold, I don't think an of the problems in CI are related to this particular PR.
/retest

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

adambkaplan commented Nov 6, 2019

Copy link
Copy Markdown
Contributor Author

/hold cancel

/retest

@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 6, 2019
@openshift-bot

openshift-bot commented Nov 6, 2019

Copy link
Copy Markdown
Contributor

/retest

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

2 similar comments
@openshift-bot

openshift-bot commented Nov 6, 2019

Copy link
Copy Markdown
Contributor

/retest

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

@openshift-bot

openshift-bot commented Nov 6, 2019

Copy link
Copy Markdown
Contributor

/retest

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

@openshift-bot

openshift-bot commented Nov 7, 2019

Copy link
Copy Markdown
Contributor

/retest

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

2 similar comments
@openshift-bot

openshift-bot commented Nov 7, 2019

Copy link
Copy Markdown
Contributor

/retest

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

@openshift-bot

openshift-bot commented Nov 7, 2019

Copy link
Copy Markdown
Contributor

/retest

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

@adambkaplan

adambkaplan commented Nov 7, 2019

Copy link
Copy Markdown
Contributor Author

/hold

Build tests need to be fixed

@openshift-ci-robot openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed lgtm Indicates that a PR is ready to be merged. labels Nov 7, 2019
@adambkaplan
React to changes
0f4a457 
* k8s.io refactors
* Move to new kube metrics registration
adambkaplan
adambkaplan commented Nov 11, 2019
View reviewed changes

@adambkaplan adambkaplan left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@soltysh updated to fix failure reporting build metrics.

Comment thread pkg/build/metrics/prometheus/metrics.go
bc.isCreated = true
})
return bc.IsCreated()
}

@adambkaplan adambkaplan Nov 11, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@openshift/openshift-team-monitoring ptal - this was pseudo-lifted from upstream's logic for initializing Prometheus core metric types.

@adambkaplan

adambkaplan commented Nov 12, 2019

Copy link
Copy Markdown
Contributor Author

/hold cancel

Metrics issue appears to have been fixed, tests green

@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 12, 2019
soltysh
soltysh approved these changes Nov 18, 2019
View reviewed changes

@soltysh soltysh left a comment

Copy link
Copy Markdown
Contributor

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 Nov 18, 2019
@openshift-ci-robot

openshift-ci-robot commented Nov 18, 2019

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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:
  • OWNERS [adambkaplan,soltysh]

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 7013ceb into openshift:master Nov 18, 2019
stlaz pushed a commit to stlaz/openshift-controller-manager that referenced this pull request Jul 29, 2021
@openshift-merge-robot
Merge pull request openshift#44 from sttts/sttts-config
0bcba11 
Shorten config resource name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mfojtik mfojtik Awaiting requested review from mfojtik
@ironcladlou ironcladlou Awaiting requested review from ironcladlou
@tnozicka tnozicka Awaiting requested review from tnozicka
1 more reviewer
@soltysh soltysh soltysh approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@soltysh soltysh

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@adambkaplan @ironcladlou @openshift-bot @openshift-ci-robot @soltysh @tnozicka @openshift-merge-robot