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

Rebase 1.16 #120

Merged
merged 688 commits into from
Oct 30, 2019
Merged

Rebase 1.16 #120

merged 688 commits into from
Oct 30, 2019

Conversation

enxebre
Copy link
Member

@enxebre enxebre commented Oct 28, 2019

Rebase:

The PR was created by first taking upstream/cluster-autoscaler-release-1.16 as the base then applying UPSTREAM: patches on top. The set of picks applied was derived from:

git log --oneline --no-merges 8a999884de8916f7fe99abb9fa63caed4997719b..openshift/master

where 8a99988 reflects the changes since our last rebase (which was upstream/cluster-autoscaler-release-1.14 #107).

And in that set of picks UPSTREAM: <carry>: openshift: Bump deps for cloudprovider/openshiftmachineapi was used to revendor using the new upstream flow which relies on cluster-autoscaler/hack/update-vendor.sh. For it to succeed and give a buildable artifact we had to relax the script to allow the go.mod-extra to set github.com/prometheus/client_golang v0.9.2 and github.com/matttproud/golang_protobuf_extensions v1.0.1.

Extra commits

Additionally an extra commit was added at the top to ensure the machine API Provider satisfy the new cloud provider interface methods:
UPSTREAM: <carry>: openshift: satisfy cloud provider interface
The interface was changed by kubernetes@9066688#diff-34ecd32e36ab8898fff1637bb1b39c2c

Additionally an extra commit was added at the top to revert kubernetes@6601bf0 and overcome kubernetes#2495

PRs Deps

This PR needs openshift/cluster-autoscaler-operator#120 for the e2e to succeed because of new RBAC requirements.

This PR needs openshift/release#5626 for unit tests to succeed
They are failing due to go 1.10 missing https://golang.org/doc/go1.12#library

Rebase process

To create the merge commit I have used the following steps:

$ git remote update
$ git checkout upstream/cluster-autoscaler-release-1.16
$ git checkout -b merge
$ git checkout openshift/master
$ echo 'merge upstream/cluster-autoscaler-release-1.16' | git commit-tree merge^{tree} -p HEAD -p merge
deadbeef12345678
$ git checkout deadbeef12345678
$ git cherry-pick ...<all-the-things>...

Picks were as follows, where the conflicts are signaled:

PICKED/CONFLICT 470bd635e UPSTREAM: <carry>: openshift: revert https://github.com/openshift/kubernetes-autoscaler/pull/113
0b4a9d514 UPSTREAM: <carry>: openshift: compare_nodegroups: Tolerate small differences in memory capacity
e0cbdb403 UPSTREAM: <carry>: openshift: Run 'make goimports' over cluster-autoscaler/cloudprovider/openshiftmachineapi
9847539cc UPSTREAM: <carry>: openshift: Extend makefile with 'make goimports' target
PICKED/CONFLICT b8ec486e1 UPSTREAM: <carry>: openshift: add custom nodeset comparator
d65c26fb2 UPSTREAM: <carry>: openshift: report MaxNodesTotal count
1b746e405 UPSTREAM: <carry>: openshift: reference k8s.io/api/core/v1 with corev1 alias
35cc7965d UPSTREAM: <carry>: openshift: Rework logic in DeleteNodes()
abbb314e0 UPSTREAM: <carry>: openshift: Switch builds to use Go 1.12
PICKED/CONFLICT 6efd972c0 UPSTREAM: <carry>: openshift: Bump deps for cloudprovider/openshiftmachineapi
c688fa6ac UPSTREAM: <carry>: openshift: add test/openshift/Makefile
b81de6560 UPSTREAM: <carry>: openshift: fix build post 1.14 rebase
86002a4a3 UPSTREAM: <carry>: openshift: simplify test setup for nodes/replicas
baa22d62f UPSTREAM: <carry>: openshift: update criteria for returning a nodegroup
7e8be6416 UPSTREAM: <carry>: openshift: add test/openshift/hack scripts
f3aa284a2 UPSTREAM: <carry>: openshift: move MaxNodesTotalReached event again
f5d5d669e UPSTREAM: <carry>: openshift: Revert "Merge pull request #90 from frobware/add-cluster-size-reached-event"
0947e6480 UPSTREAM: <carry>: openshift: move maxnodes total event
eb75e6d7d UPSTREAM: <carry>: openshift: Add joelsmith and sjenning to VPA OWNERS file
beeedef12 UPSTREAM: <carry>: openshift: Add OpenShift VPA image builds
1dade0d72 UPSTREAM: <carry>: openshift: prioritize providerID in calls to Nodes()
f25a6aa17 UPSTREAM: <carry>: openshift: add test to search using annotation
4ec5b6887 UPSTREAM: <carry>: openshift: Prioritize machine search using provider ID
bf7bf2354 UPSTREAM: <carry>: openshift: add findMachineByNodeProviderID
e030dd3d3 UPSTREAM: <carry>: openshift: add index to machine informer
3412cfc98 UPSTREAM: <carry>: openshift: simplify config creation
a63568c3c UPSTREAM: <carry>: openshift: record max-nodes-total event
68c4c33e3 UPSTREAM: <carry>: openshift: disable MachineDeployment in normal operation
a83320a7b UPSTREAM: <carry>: openshift: shorten test helper names
909396c97 UPSTREAM: <carry>: openshift: add debugFormat const
dfb77c5a4 UPSTREAM: <carry>: openshift: move machineAnnotationKey constant
ce3e9a721 UPSTREAM: <carry>: openshift: rework controller test setup
2b97154d1 UPSTREAM: <carry>: openshift: rework nodegroup test setup
bdc7ed988 UPSTREAM: <carry>: openshift: rework provider test setup
bc4db8435 UPSTREAM: <carry>: openshift: add new test helper
a0ffce84f UPSTREAM: <carry>: openshift: expose nodegroup type for testing
d816c806e UPSTREAM: <carry>: openshift: simplify controller node lookup tests
d7b961851 UPSTREAM: <carry>: openshift: simplify TestControllerNodeGroupForNodeWithMissingMachineOwner
ff81185b6 UPSTREAM: <carry>: openshift: remove TestControllerNodeGroupsSizes
f4ad36cdf UPSTREAM: <carry>: openshift: simplify TestControllerMachinesInMachineSet
219d52859 UPSTREAM: <carry>: openshift: simplify TestControllerFindNodeByNodeName
a4563cb4e UPSTREAM: <carry>: openshift: simplify TestControllerFindMachineByNodeProviderID
56ab19993 UPSTREAM: <carry>: openshift: simplify TestControllerFindMachineOwner
efcb4e0e1 UPSTREAM: <carry>: openshift: fix lint issue machineapi_provider
5a0d21103 UPSTREAM: <carry>: openshift: Add fmt, lint, vet scripts/Makefile
PICKED/CONFLICT 35b37a052 UPSTREAM: <carry>: openshift: expose KubeConfigPath in CA options
6a251a307 UPSTREAM: <carry>: openshift: cloudprovider builder updates for 1.13
PICKED/CONFLICT 4ecc1d233 UPSTREAM: <carry>: openshift: cloudprovider updates for 1.13
afa3ff275 UPSTREAM: <carry>: openshift: add unit test TestNodeGroupDecreaseTargetSize
2f11664a8 UPSTREAM: <carry>: openshift: add unit test TestNodeGroupIncreaseSize
820cea0ad UPSTREAM: <carry>: openshift: create git history verification script
587a0a5bc UPSTREAM: <carry>: openshift: check for explicit errors in resize tests
be823b0bf UPSTREAM: <carry>: openshift: move all test utility functions
817bd97bd UPSTREAM: <carry>: openshift: remove parallel tests
a11169ddf UPSTREAM: <carry>: openshift: simplify TestProviderConstructorProperties
2a43ac621 UPSTREAM: <carry>: openshift: add spec to clusterTestConfig
acaebb962 UPSTREAM: <carry>: openshift: force test namespace ToLower()
8e96d1ac1 UPSTREAM: <carry>: openshift: remove unused makeMachineOwner()
f4bdec1ab UPSTREAM: <carry>: openshift: remove t.Helper() in test helpers
14f6d94d9 UPSTREAM: <carry>: openshift: Rework TestControllerNodeGroupForNodeLookup
025c4e64a UPSTREAM: <carry>: openshift: Rework utils test funcs
548aef244 UPSTREAM: <carry>: openshift: Rework TestControllerFindMachineByID
e7499baee UPSTREAM: <carry>: openshift: Rework TestControllerNodeGroups
368ba1b25 UPSTREAM: <carry>: openshift: Rework TestNodeGroupDeleteNodes
0368ecb33 UPSTREAM: <carry>: openshift: Rework TestNodeGroupResize
216538a54 UPSTREAM: <carry>: openshift: Rework TestNodeGroupNewNodeGroup
59c5273c8 UPSTREAM: <carry>: openshift: remove TODO
e7184e4b9 UPSTREAM: <carry>: openshift: openshiftmachineapi: remove unused fields
346070a04 UPSTREAM: <carry>: openshift: Remove old test functions
6c294f2d5 UPSTREAM: <carry>: openshift: return no nodegroup when scaling bounds == 0
53626b2ad UPSTREAM: <carry>: openshift: create utility functions
b2811c128 UPSTREAM: <carry>: openshift: validate node membership in DeleteNodes()
22c8f9347 UPSTREAM: <carry>: openshift: log nodegroup discovery at level 4
9a17ba0c1 UPSTREAM: <carry>: openshift: address review comments
13ef18dd2 UPSTREAM: <carry>: openshift: openshiftmachineapi: add feature gate for MachineDeployment support
54908968f UPSTREAM: <carry>: openshift: add unit test for nodegroup
751691069 UPSTREAM: <carry>: openshift: add unit test for provider
1c3006351 UPSTREAM: <carry>: openshift: add unit test for controller
0ebf27bd8 UPSTREAM: <carry>: openshift: add unit test for utils
a3c490977 UPSTREAM: <carry>: openshift: MachineDeployment implementation of ScalableResource
e7f222689 UPSTREAM: <carry>: openshift: MachineSet implementation of ScalableResource
468764cbc UPSTREAM: <carry>: openshift: Add interface ScalableResource
22cc1b4e2 UPSTREAM: <carry>: openshift: Revert to whitebox testing
cf756c02d UPSTREAM: <carry>: openshift: Add MachineDeployment informers to controller
c8752bd63 UPSTREAM: <carry>: openshift: cloudprovider: pivot to machine.openshift.io/v1beta1
d71287be3 UPSTREAM: <carry>: openshift: assign ownership to cloud team
bf71a0d67 UPSTREAM: <carry>: openshift: Remove machineController.MachineSets() as it is not called
2baa1b0ea UPSTREAM: <carry>: openshift: Add unit test for MachinesInMachineSet()
e8bb3a916 UPSTREAM: <carry>: openshift: Add unit test for findMachineByNodeProviderID()
19c6a640f UPSTREAM: <carry>: openshift: Add unit test for findMachineOwner()
51f48149e UPSTREAM: <carry>: openshift: Add unit test for findNodeByNodeName()
f7c9db1e5 UPSTREAM: <carry>: openshift: Add unit test for findMachine()
342b52a52 UPSTREAM: <carry>: openshift: Remove obsolete usage of "machine" annotation
6260a467c UPSTREAM: <carry>: openshift: utils: add unit tests for clusterapi_utils.go
13be13650 UPSTREAM: <carry>: openshift: Use node.Spec.ProviderID instead of node.Name
27b6ee75b UPSTREAM: <carry>: openshift: Decouple nodegroup via dependency injection
26dcf0944 UPSTREAM: <carry>: openshift: Use 'machine' for machine parameter names
21c6b0f0a UPSTREAM: <carry>: openshift: Rename field provider.clusterapi to clusterapiClient
d56b05e9b UPSTREAM: <carry>: openshift: Use NamespaceAll in lieu of ""
2f796423b UPSTREAM: <carry>: openshift: Rename clusterController to machineController
c91d57a1a UPSTREAM: <carry>: openshift: Additional logging in provider.NodeGroups()
1b8f4aac4 UPSTREAM: <carry>: openshift: Move min/max constants to clusterapi_utils.go
92ac3ce3c UPSTREAM: <carry>: openshift: Switch to informers for observing machines and machinesets
a0b82709a UPSTREAM: <carry>: openshift: cluster-api switch to annotations
be3b8e840 UPSTREAM: <carry>: openshift: Add a RHEL7 dockerfile and standarize format
ca73a66de UPSTREAM: <carry>: openshift: initial cluster-api provider implementation
83db04b86 UPSTREAM: <carry>: openshift: cluster-autoscaler.spec: set golang_version to 1.10
a4903d792 UPSTREAM: <carry>: openshift: cluster-autoscaler.spec: bump golang_version to 1.10.4
532d2e88e UPSTREAM: <carry>: openshift: Fix spec file to be consistent
3d619d08b UPSTREAM: <carry>: openshift: Bump embedded tools
cc885725d UPSTREAM: <carry>: openshift: Fix the spec and hack scripts so the package can be built in both build systems
PICKED/CONFLICT 647555606 UPSTREAM: <carry>: openshift: Add openshift/release Makefile and hack scripts
abd595981 UPSTREAM: <carry>: openshift: Add dockerfile for cluster autoscaler.
302c7ab11 UPSTREAM: <carry>: openshift: Add spec file for cluster-autoscaler.

k8s-ci-robot and others added 30 commits July 16, 2019 02:07
Run VPA e2e tests for both apis even if one fails
add test case for controller_fetcher
correct comments
typo
add test case for controller_fetcher and cluster_feeder
Currently we read certificate files into a buffer of 5000 bytes. This
sets a max size for our certificates; above this max, we will get
invalid certs, most likely, leading to runtime bad certificate errors.
Use ioutil for certificate file reading.
- SubscriptionID placeholder does not match documentation or surrounding placeholders
Add support for cronjobs in VPA
Update c5,i3en,m5,r5 instance types
xrange() was removed in Python 3 in favor of range()
Fix typo in autoscaler azure example
…rash

nodeGroup judy IsNil to avoid crashed
…erver-over-heapster

Use Metrics Server instead of Heapster in the FAQ
Update VPA dependencies to k8s 1.14.3
Switch VPA examples to use apps/v1 API for Deployment
…milarity_rules

fix: ignore agentpool label when looking for similar node groups with Azure provider
…onfig_readme

Add multinode config in readme.
"delete node" has a specific meaning in the context of the Kubernetes
API: deleting the node object. However, the cluster-autoscaler never
does that; it terminates the underlying instance and expects the
[cloud-node-controller](https://kubernetes.io/docs/concepts/architecture/cloud-controller/#node-controller)
to remove the corresponding node from Kubernetes.

Replace all mentions of "deleting" a node with "terminating" to
disambiguate this.

Signed-off-by: Matthias Rampke <mr@soundcloud.com>
This had me stumped for the better part of a day. While the
cluster-autoscaler "deletes" nodes, it does not actually delete the Node
object from the Kubernetes API.

In normal operations, with a well-configured cluster, this is a minor
point; however, when debugging why nodes do not get deleted, the
inconsistent terminology can be a major headache. This FAQ entry should
clarify the difference for anyone who needs to know.

Signed-off-by: Matthias Rampke <mr@soundcloud.com>
…examples

Add required selector to VPA deployment examples for apps/v1
up when there was a multizonal pool with number of nodes exceeding limit for one zone.
…nodes

cluster-autoscaler FAQ: clarify what "deleting nodes" means in this context
@openshift-ci-robot openshift-ci-robot added the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label Oct 28, 2019
@enxebre
Copy link
Member Author

enxebre commented Oct 28, 2019

/test unit

@enxebre
Copy link
Member Author

enxebre commented Oct 28, 2019

Unit tests needs openshift/release#5626
They are failing likely to go 1.10 missing https://golang.org/doc/go1.12#library

# k8s.io/autoscaler/cluster-autoscaler/vendor/k8s.io/component-base/cli/flag
cluster-autoscaler/vendor/k8s.io/component-base/cli/flag/ciphersuites_flag.go:80:18: undefined: tls.VersionTLS13
[ERROR] PID 17: hack/test-go.sh:190: `go test -i ${gotest_flags} ${test_packages}` exited with status 2.
[INFO]          Stack Trace:
[INFO]            1: hack/test-go.sh:190: `go test -i ${gotest_flags} ${test_packages}`
[INFO]   Exiting with code 2.
[ERROR] hack/test-go.sh exited with code 2 after 00h 01m 09s
sh-4.2$```

@enxebre
Copy link
Member Author

enxebre commented Oct 29, 2019

/test e2e-aws-operator

@enxebre
Copy link
Member Author

enxebre commented Oct 29, 2019

Needs openshift/cluster-autoscaler-operator#120 for the e2e to succeed

@enxebre
Copy link
Member Author

enxebre commented Oct 29, 2019

@enxebre
Copy link
Member Author

enxebre commented Oct 29, 2019

/verify-owners

update cluster-autoscaler OWNERS and remove owners which do not belong the openshift org so also CI is happy.
@enxebre
Copy link
Member Author

enxebre commented Oct 29, 2019

/verify-owners

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label Oct 29, 2019
@enxebre
Copy link
Member Author

enxebre commented Oct 29, 2019

/test e2e-aws-operator

@enxebre
Copy link
Member Author

enxebre commented Oct 30, 2019

Once this gets refreshed through CI openshift/release#5626 units should go green
/test unit

@enxebre
Copy link
Member Author

enxebre commented Oct 30, 2019

/test unit

2 similar comments
@droslean
Copy link
Member

/test unit

@enxebre
Copy link
Member Author

enxebre commented Oct 30, 2019

/test unit

Copy link

@bison bison left a comment

Choose a reason for hiding this comment

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

This is not possible to review in a meaningful way. Process sounds right. LGTM if the tests pass.

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2019
@openshift-merge-robot openshift-merge-robot merged commit d45f94d into openshift:master Oct 30, 2019
@enxebre enxebre mentioned this pull request Jan 14, 2020
@enxebre enxebre mentioned this pull request Mar 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.