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.19.0 #164

Merged
merged 355 commits into from Aug 7, 2020
Merged

Conversation

elmiko
Copy link

@elmiko elmiko commented Aug 5, 2020

This commit rebases the autoscaler on top of the Kubernetes/Autoscaler 1.19.0 release. There are several commits that we carry on top of the upstream autoscaler and the rebase process allows us to preserve those. Here is a description of the process I used to create this PR.

(inspired by the commit description for the 1.18 rebase. pr #139)

Process

First we need to identify the carry commits that we currently have, this is done against our previous rebase to catch new changes. Once identified we will drop commits which have merged upstream and only carry unique commits. (see below for the carried and dropped commits).

Identify carry commits:

git log --oneline --no-merges a1ddf4795100a7ac3a966ad2204ace7235c05ff1..openshift/master

where a1ddf4 reflects the changes since our last rebase (1.18.0). this is the
list of commits we will need to apply onto the new upstream version of the
autoscaler. ideally, some of these commits can be dropped.

After identifying the carry commits, the next step is to create the new commit-tree that will be used for the rebase and then cherry pick the carry commits into the new branch. The following commands cover these steps:

$ git remote update # make sure we update our refs
$ git checkout db76938c649a3717ca4fd8caf2944bf8ef340ecb # cluster-autoscaler-1.19.0
$ git checkout -b merge-tmp # create a temporary branch for our merge commit
$ git checkout openshift/master # we want to be at the tip of the openshift master branch when we run the next command
$ echo 'merge upstream/cluster-autoscaler-1.19.0' | git commit-tree merge-tmp^{tree} -p HEAD -p merge-tmp -F - # create a new merge commit for our history
deadbeef12345678 # id of new merge commit
$ git branch merge-1.19.0 deadbeef12345678 # create a new branch for the cherry-pick work
$ git checkout merge-1.19.0
$ git cherry-pick <carry commits> # cherry pick the needed commits into the new branch

With the merge-1.19.0 branch in place, I cherry picked the carry commits which applied, resolved merge conflicts, and finally tested the resulting tree against the unit test and end-to-end suite.

Carried Commits

These commits are for features which have not yet been accepted upstream, are integral to our CI platform, or are specific to the releases we create for OpenShift.

4c60cef91 UPSTREAM: <carry>: openshift: Fallback to Status Replicas if Replicas nil when listing NodeGroups
feac275be UPSTREAM: <carry>: Convert the mem value consistently with other providers
487674be0 UPSTREAM: <carry>: openshift: Fallback to status if replicas nil in spec
fe618225e UPSTREAM: <carry>: openshift: Implement scale from zero
4feb7be3d UPSTREAM: <carry>: openshift: Rename FailureMessage to ErrorMessage
33715ff54 UPSTREAM: <carry>: openshift: update cluster-autoscaler OWNERS
a0492ee84 UPSTREAM: <carry>: openshift: Add OpenShift VPA image builds
55f8c76dc UPSTREAM: <carry>: openshift: Revert "Adding config for event filtering"
e5d75086b UPSTREAM: <carry>: openshift: Hardcode API group and annotations to machine.openshift.io
5db2eabe7 UPSTREAM: <carry>: Bump scripts go version to 1.13 and update path to cloudprovider/clusterapi
3dc78fcc6 UPSTREAM: <carry>: openshift: Extend makefile with 'make goimports' target
97681c560 UPSTREAM: <carry>: Fix git commit message verification script
561763adb UPSTREAM: <carry>: openshift: Switch builds to use Go 1.12
8bee2fe79 UPSTREAM: <carry>: openshift: create git history verification script
b8c5d9d6b UPSTREAM: <carry>: openshift: Add fmt, lint, vet scripts/Makefile
c55d13e35 UPSTREAM: <carry>: openshift: Add a RHEL7 dockerfile and standarize format
c81123283 UPSTREAM: <carry>: openshift: cluster-autoscaler.spec: set golang_version to 1.10
3bc1590b1 UPSTREAM: <carry>: openshift: cluster-autoscaler.spec: bump golang_version to 1.10.4
cdae7f285 UPSTREAM: <carry>: openshift: Fix spec file to be consistent
dacc7035f UPSTREAM: <carry>: openshift: Bump embedded tools
cef5e8a1d UPSTREAM: <carry>: openshift: Fix the spec and hack scripts so the package can be built in both build systems
320c648ce UPSTREAM: <carry>: openshift: Add openshift/release Makefile and hack scripts
4a7b05e02 UPSTREAM: <carry>: openshift: Add dockerfile for cluster autoscaler.
a34cf0a0d UPSTREAM: <carry>: openshift: Add spec file for cluster-autoscaler.

Dropped Commits

These commits were carried over the 1.18.x release history and represent work that has been accepted upstream.

e036217d8 UPSTREAM: <carry>: openshift: Support alias openshift-machine-api as cloud provider
90751c4b9 UPSTREAM: 3124: openshift: Allow small tolerance on memory capacity when comparing nodegroups
62834aa0a UPSTREAM: <carry>: remove redundant error checks in mark/unmark deletion functions
3ec206282 UPSTREAM: 3125: openshift: Rewrite DeleteNodesTwice test to check API not TargetSize
9e227899e UPSTREAM: <carry>: Add mutex to DeleteNodes
6f34947d3 UPSTREAM: <carry>: Get replicas always from API server
e7f689b4c UPSTREAM: <carry>: Compare against minSize in deleteNodes()
4903b8b15 UPSTREAM: 3034: openshift: Improve delete node mechanisms
e6900ffe1 UPSTREAM: 3057: openshift: Do not normalize Node IDs outside of CAPI provider
d2afb1111 UPSTREAM: 2983: openshift: Use FailureMessage instead of Phase to determine failed Machines
3f7cc57fe UPSTREAM: 2983: openshift: Add testing for fake provider IDs
028d97a2e UPSTREAM: 2983: openshift: Provide fake proivder IDs for failed machines
a6461e8bf UPSTREAM: <carry>: openshift: report MaxNodesTotal count
dfe75a4fd UPSTREAM: <carry>: openshift: Let the controller move on if machineDeployment resource does not exist.

yoyota and others added 30 commits April 24, 2020 12:20
slusterState -> clusterState
The following things changed in scheduler and needed to be fixed:
* NodeInfo was moved to schedulerframework
* Some fields on NodeInfo are now exposed directly instead of via getters
* NodeInfo.Pods is now a list of *schedulerframework.PodInfo, not *apiv1.Pod
* SharedLister and NodeInfoLister were moved to schedulerframework
* PodLister was removed
Update vendor & fix breaking scheduler changes
Small update to reflect that usage of multi-AZ ASGs are supported, and the relevant considerations for using them. a couple of other small changes in the MixedInstancesPolicy section as well.
Co-Authored-By: Guy Templeton <guyjtempleton@googlemail.com>
Add api-approved label to the Vertical Pod Autoscaler API
This change adds a readme which describes the basic operation of the
cluster-api provider.
add EphemeralStorage in group template
Update readme with ASG considerations and MixedInstancesPolicy notes
fixing wrong syntax in links that were merged in kubernetes#3096 (had a space between ']' and '(' so the links didn't work)
Typo - Assert using the right containers
Signed-off-by: Marc Sensenich <sensenichm91@gmail.com>
Signed-off-by: Marc Sensenich <sensenichm91@gmail.com>
This is needed for map containerName: aggregatedState to contain
complete information (not only samples, but also resource policy), which
can be later processed downstream. In particular, it is needed for VPA
controlled resources.
e2-micro and e2-small have allocatable set too high resulting in
overcommit. To make cluster autoscaler prefer e2-medium given the choice
between the three machine types, the prices for e2-micro and e2-small
are artificially set to be higher than the price of e2-medium.
Add prices for all machine types, adjust e2 fractional VM pricing
@elmiko
Copy link
Author

elmiko commented Aug 7, 2020

/test e2e-azure-operator

@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 7, 2020

@elmiko: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/git-history 88e10aa link /test git-history

Full PR test history. Your PR dashboard.

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.

@elmiko
Copy link
Author

elmiko commented Aug 7, 2020

/test e2e-azure-operator

@enxebre
Copy link
Member

enxebre commented Aug 7, 2020

Albertos-MacBook-Pro:autoscaler@albertogarla $ git log --oneline openshift/master..elmiko/openshift-rebase-1.19 --reverse
7111717ce Propagate scaling mode to containers. Correctly update ResourcePolicy and UpdatePolicy on update.
cfe5d7bc1 Fix misspelled parenthesis
19a78f698 Add some more documentation to clarify how labels and GPUs work with the ClusterAutoScaler on AWS
c53810cc0 Support arbitrary custom resource building template
.....
git log --oneline upstream/cluster-autoscaler-release-1.18..upstream/cluster-autoscaler-release-1.19 --no-merges --reverse
7111717ce Propagate scaling mode to containers. Correctly update ResourcePolicy and UpdatePolicy on update.
cfe5d7bc1 Fix misspelled parenthesis
19a78f698 Add some more documentation to clarify how labels and GPUs work with the ClusterAutoScaler on AWS
c53810cc0 Support arbitrary custom resource building template
....

Procedure seems ok to me.
/approve

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre

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-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 7, 2020
@JoelSpeed
Copy link

/lgtm
/hold

@elmiko I assume you're happy for this to merge, but I'll hold until you're online just in case, unhold if you're ready

@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 Aug 7, 2020
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 7, 2020
@elmiko
Copy link
Author

elmiko commented Aug 7, 2020

@JoelSpeed i'm good to merge this, i don't think i would do any other adjustments for 1.19. for 1.20, we can target squashing all those commits Alberto mentioned.

@JoelSpeed
Copy link

/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 Aug 7, 2020
@openshift-merge-robot openshift-merge-robot merged commit 76b6b5d into openshift:master Aug 7, 2020
@elmiko elmiko deleted the openshift-rebase-1.19.0 branch January 8, 2021 13:55
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet