BUG 1815039: Retrieve the latest status of SriovNetworkNodeState CR when update is… #175

pliurh · 2020-03-31T08:52:50Z

… triggered

openshift-ci-robot · 2020-03-31T08:52:58Z

@pliurh: This pull request references Bugzilla bug 1815039, which is invalid:

expected the bug to target the "4.5.0" release, but it targets "4.4.z" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

BUG 1815039: Retrieve the latest status of SriovNetworkNodeState CR when update is…

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.

pliurh · 2020-03-31T08:54:15Z

/bugzilla refresh

openshift-ci-robot · 2020-03-31T08:54:27Z

@pliurh: This pull request references Bugzilla bug 1815039, 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.5.0) matches configured target release for branch (4.5.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

/bugzilla refresh

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.

zshi-redhat · 2020-04-07T07:55:38Z

pkg/daemon/daemon.go

@@ -281,9 +281,24 @@ func (dn *Daemon) nodeStateChangeHandler(old, new interface{}) {
 	}
 	reqReboot := false
 	reqDrain := false
+	if len(dn.LoadedPlugins) == 0 {


Should this be already loaded when nodeChange is called?

This is to ensure that the plugins will only be loaded once. Normally, the plugins will be loaded when the daemon starts, and nodeStateAddHandler will invoke nodeStateChangeHandler for the first time.

I removed the logic at the latest commit.

zshi-redhat · 2020-04-07T08:57:30Z

pkg/daemon/daemon.go

+		}
+	}
+	// Get the latest NodeState
+	latestState, err := dn.client.SriovnetworkV1().SriovNetworkNodeStates(namespace).Get(dn.name, metav1.GetOptions{})


Can we guarantee latestState is the latest? I think there is a chance that daemon writer not updating node state immediately after policy is deleted.

All the CR change event will be put in the workqueue, and be processed one after another. So I add a logic to check if the spec of the newState is identical with the latestState, if not, the event will be skipped. It makes sure that only the event latestState will be processed.

pliurh · 2020-04-08T13:58:06Z

/hold
I need to refactor this PR a bit.

pliurh · 2020-04-10T09:05:15Z

/unhold

zshi-redhat · 2020-04-14T02:18:22Z

pkg/daemon/daemon.go

-	if newState.GetObjectMeta().GetGeneration() == oldState.GetObjectMeta().GetGeneration() {
+	glog.V(2).Infof("nodeStateChangeHandler(): new generation is %d", newState.GetObjectMeta().GetGeneration())
+	// Get the latest NodeState
+	latestState, err := dn.client.SriovnetworkV1().SriovNetworkNodeStates(namespace).Get(dn.name, metav1.GetOptions{})


shall it use retry here to avoid transient failure (as the error handling is exiting the daemon)?

… triggered

pliurh · 2020-04-15T15:28:41Z

/test e2e-aws

pliurh · 2020-04-16T03:05:44Z

/test e2e-aws

zshi-redhat · 2020-04-16T09:41:40Z

/lgtm

pliurh · 2020-04-17T02:16:11Z

/test e2e-aws

pliurh · 2020-04-17T04:11:38Z

/test e2e-aws

openshift-ci-robot · 2020-04-17T05:45:02Z

@pliurh: All pull requests linked via external trackers have merged: openshift/sriov-network-operator#175. Bugzilla bug 1815039 has been moved to the MODIFIED state.

In response to this:

BUG 1815039: Retrieve the latest status of SriovNetworkNodeState CR when update is…

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.

This Commit is base on openshift#175 Signed-off-by: Sebastian Sch <sebassch@gmail.com>

zshi-redhat · 2020-05-11T10:00:49Z

/cherry-pick release-4.4

openshift-cherrypick-robot · 2020-05-11T10:00:56Z

@zshi-redhat: #175 failed to apply on top of branch "release-4.4":

error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	pkg/daemon/daemon.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/daemon/daemon.go
CONFLICT (content): Merge conflict in pkg/daemon/daemon.go
Patch failed at 0001 Retrieve the latest status of SriovNetworkNodeState CR when update is triggered

In response to this:

/cherry-pick release-4.4

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.

This commit is base on the CNF repo change openshift-kni/cnf-features-deploy#196 This commit also remove the check for logs as it's not needed anymore the bug was fixed by openshift#175 Also here we fix some test issues related to the policyName field Signed-off-by: Sebastian Sch <sebassch@gmail.com>

Use GlobalMgr for SriovNetwork controllers

openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Mar 31, 2020

openshift-ci-robot requested review from s1061123 and squeed March 31, 2020 08:53

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Mar 31, 2020

openshift-ci-robot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Mar 31, 2020

pliurh requested a review from zshi-redhat March 31, 2020 08:54

pliurh assigned zshi-redhat Mar 31, 2020

zshi-redhat reviewed Apr 7, 2020

View reviewed changes

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 8, 2020

pliurh force-pushed the latest_state branch from 33a25ca to 757d116 Compare April 10, 2020 08:59

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 10, 2020

SchSeba mentioned this pull request Apr 13, 2020

Make tests more stable #185

Merged

zshi-redhat reviewed Apr 14, 2020

View reviewed changes

pliurh force-pushed the latest_state branch from 757d116 to c9f15b4 Compare April 15, 2020 09:49

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 15, 2020

pliurh force-pushed the latest_state branch from c9f15b4 to 88d4fb8 Compare April 15, 2020 12:08

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 15, 2020

pliurh force-pushed the latest_state branch from 88d4fb8 to 9654d50 Compare April 15, 2020 12:16

Retrieve the latest status of SriovNetworkNodeState CR when update is…

77d1535

… triggered

pliurh force-pushed the latest_state branch from 9654d50 to 77d1535 Compare April 15, 2020 12:19

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 16, 2020

openshift-merge-robot merged commit 45b9f64 into openshift:master Apr 17, 2020

SchSeba added a commit to SchSeba/sriov-network-operator that referenced this pull request May 10, 2020

Change the logs search in the test

c3ef27a

This Commit is base on openshift#175 Signed-off-by: Sebastian Sch <sebassch@gmail.com>

SchSeba mentioned this pull request May 10, 2020

Bug 1834371: Change the logs search in the test #205

Merged

SchSeba pushed a commit to SchSeba/sriov-network-operator that referenced this pull request Aug 17, 2021

Merge pull request openshift#175 from oribon/fix_mgr

568a77f

Use GlobalMgr for SriovNetwork controllers

BUG 1815039: Retrieve the latest status of SriovNetworkNodeState CR when update is… #175

BUG 1815039: Retrieve the latest status of SriovNetworkNodeState CR when update is… #175

pliurh commented Mar 31, 2020

openshift-ci-robot commented Mar 31, 2020

pliurh commented Mar 31, 2020

openshift-ci-robot commented Mar 31, 2020

zshi-redhat Apr 7, 2020

pliurh Apr 7, 2020

pliurh Apr 10, 2020

zshi-redhat Apr 7, 2020

pliurh Apr 10, 2020

pliurh commented Apr 8, 2020

pliurh commented Apr 10, 2020

zshi-redhat Apr 14, 2020

pliurh Apr 15, 2020

pliurh commented Apr 15, 2020

pliurh commented Apr 16, 2020

zshi-redhat commented Apr 16, 2020

pliurh commented Apr 17, 2020

pliurh commented Apr 17, 2020

openshift-ci-robot commented Apr 17, 2020

zshi-redhat commented May 11, 2020

openshift-cherrypick-robot commented May 11, 2020

BUG 1815039: Retrieve the latest status of SriovNetworkNodeState CR when update is… #175

BUG 1815039: Retrieve the latest status of SriovNetworkNodeState CR when update is… #175

Conversation

pliurh commented Mar 31, 2020

openshift-ci-robot commented Mar 31, 2020

pliurh commented Mar 31, 2020

openshift-ci-robot commented Mar 31, 2020

zshi-redhat Apr 7, 2020

Choose a reason for hiding this comment

pliurh Apr 7, 2020

Choose a reason for hiding this comment

pliurh Apr 10, 2020

Choose a reason for hiding this comment

zshi-redhat Apr 7, 2020

Choose a reason for hiding this comment

pliurh Apr 10, 2020

Choose a reason for hiding this comment

pliurh commented Apr 8, 2020

pliurh commented Apr 10, 2020

zshi-redhat Apr 14, 2020

Choose a reason for hiding this comment

pliurh Apr 15, 2020

Choose a reason for hiding this comment

pliurh commented Apr 15, 2020

pliurh commented Apr 16, 2020

zshi-redhat commented Apr 16, 2020

pliurh commented Apr 17, 2020

pliurh commented Apr 17, 2020

openshift-ci-robot commented Apr 17, 2020

zshi-redhat commented May 11, 2020

openshift-cherrypick-robot commented May 11, 2020