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

Bug 1954309: Rebase to upstream 0.47.0 #191

Merged
merged 11 commits into from
Jun 8, 2021

Conversation

cybertron
Copy link
Member

kubernetes-nmstate 0.47.0 rebase process

Inspired by the commit description for openshift/coredns#52

Identify carry commits:

git log --oneline --no-merges v0.47.0..openshift/master

Note that the UPSTREAM: <carry>: openshift: prefix was added to any new commits to carry since the previous rebase. This will make new carry commits easy to identify alongside prior carry commits.

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:

Process

$ git remote update # make sure we update our refs
$ git checkout v0.47.0 # upstream kubernetes-nmstate 0.47.0 commit tag
$ git checkout -b merge-tmp # create a branch to do our merge work from
$ git checkout openshift/master # we want to be at the tip of the openshift master branch when we run the next command
$ echo 'merge nmstate/kubernetes-nmstate v0.47.0' | git commit-tree merge-tmp^{tree} -p HEAD -p merge-tmp -F -
deadbeef12345678 # id of new merge commit, output by the previous command
$ git branch merge-0.47.0 deadbeef12345678 # create a new branch for the cherry-pick work
$ git checkout merge-0.47.0 # make sure we are on the proper branch
$ git cherry-pick <carry commits>

My 0.47.0 merge commit from the above process is (af57ae1).

With the merge-0.47.0 branch in place, I cherry picked the carry commits. Merge conflicts were resolved on an individual commit basis when necessary. Some carry commits may have been squashed to reduce the number of cherry-picks in future rebases. If there are any questions please let me know.

Carried Commits (applied bottom to top / oldest first):

29502d99 UPSTREAM: <carry>: Add manifests for 4.8
8dfe9853 UPSTREAM: <carry>: Update Dockerfile images to match ART
c016e6fe UPSTREAM: <carry>: Add manifests for 4.7
09d2d1d2 UPSTREAM: <carry>: Add midstream approvers and reviewers

qinqon and others added 11 commits April 9, 2021 12:36
If kubernetes-nmstate is deployed at a cluster with not compatible nodes
(like NetworkManager not started, bad version or not installed) the
kubernetes-nmstate-handler start without issue. This change add a
readiness probe that runs "nmstatectl show" to check if nmstatectl is
functional at a basic level.

Signed-off-by: Quique Llorente <[email protected]>
At the new release-notes command version (it was bumped after some "make
vendor") does not need "--release-version" flag and the "--github-org"
and "--github-repo" arg have being renamed to "--org" and "--repo".

Signed-off-by: Quique Llorente <[email protected]>
* Use github.com/gofrs/flock to lock handler

The nmstate-handler has a lock that make it wait if another handler is
already running in the system, but the library used for that does not
work well with containers. This change replace that library with
different one that uses golang syscall.Flock behind the scine [1] and
that works fine with containers sharing node volume.

[1] github.com/nightlyone/lockfile.

Signed-off-by: Quique Llorente <[email protected]>

* Replace handler readiness probe

The nmstate handler is marked as ready even if it was not able to take
ownership of the lock, this change runs nmstatectl and touchs a file
after gaining lock ownership so the readiness probe can check both
things.

Signed-off-by: Quique Llorente <[email protected]>

* Test nmstate handler lock mechanism

The nmstate handler don't get ready if another handler is running at the
same node. This change add an e2e tests to check that this is working.

Signed-off-by: Quique Llorente <[email protected]>
The previous order of these calls resulted in the container exiting
before the log message was output, which made it impossible to debug
what went wrong. This just changes the order so the message is logged
and then the process exits.

Signed-off-by: Ben Nemec <[email protected]>
* Reorder nnpc controller functions

The first thing that reader is presumably interested in when opening
nodenetworkconfigurationpolicy_controller.go file is the Reconcile loop.
In this commit, I'm moving the Reconcile loop to the top of the file,
and reordering the helper functions so that the reader can see the
details in the same order in which these helper functions are called in
Reconcile.

Signed-off-by: Radim Hrazdil <[email protected]>

* Remove unused method enactmentsCountByPolicy

enactmentsCountByPolicy is not used, let's remove it.

Signed-off-by: Radim Hrazdil <[email protected]>
The internal Networking team needs to be able to approve and review.

Signed-off-by: Brad P. Crochet <[email protected]>
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cybertron

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 May 5, 2021
@cybertron
Copy link
Member Author

Note that although we already did a rebase for 4.8, this one pulls in some critical bug fixes that were discovered after the previous one.

@cybertron cybertron changed the title Rebase to upstream 0.47.0 Bug 1954309: Rebase to upstream 0.47.0 May 5, 2021
@openshift-ci-robot openshift-ci-robot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels May 5, 2021
@openshift-ci-robot
Copy link

@cybertron: This pull request references Bugzilla bug 1954309, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.8.0) matches configured target release for branch (4.8.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla ([email protected]), skipping review request.

In response to this:

Bug 1954309: Rebase to upstream 0.47.0

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.

@yboaron
Copy link

yboaron commented Jun 8, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 8, 2021
@openshift-merge-robot openshift-merge-robot merged commit 7b5332c into openshift:master Jun 8, 2021
@openshift-ci
Copy link

openshift-ci bot commented Jun 8, 2021

@cybertron: All pull requests linked via external trackers have merged:

Bugzilla bug 1954309 has been moved to the MODIFIED state.

In response to this:

Bug 1954309: Rebase to upstream 0.47.0

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.

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. bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants