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
feat: bump gwapi to v0.6.0 #853
Conversation
Codecov Report
@@ Coverage Diff @@
## main #853 +/- ##
==========================================
- Coverage 64.06% 64.00% -0.06%
==========================================
Files 50 51 +1
Lines 6486 6873 +387
==========================================
+ Hits 4155 4399 +244
- Misses 2075 2200 +125
- Partials 256 274 +18
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
looks like the core issue is interpretation of mapping of Observed Generation to Status Conditions gateway/internal/status/conditions.go Line 127 in 085e6fa
gateway/internal/gatewayapi/contexts.go Line 484 in 085e6fa
|
from the Condition spec: https://github.com/kubernetes/apimachinery/blob/6c409361e35e40e38c4056ba0b86647d4244c047/pkg/apis/meta/v1/types.go#L1480-L1485 even if the type/reason/status haven't changed, the observed generation should be updated to make clear the status condition applies to the latest version of the resource |
thanks for sharing that @sunjayBhatia ! , the spec makes it clear that the condition is considered out of date if the observed generation is older gateway/internal/status/conditions.go Line 127 in 085e6fa
gateway/internal/gatewayapi/contexts.go Line 484 in 085e6fa
|
c50c1e1
to
3a0c333
Compare
Signed-off-by: bitliu <bitliu@tencent.com>
3a0c333
to
d8fc059
Compare
Done. |
74434ff
to
d8fc059
Compare
@@ -495,6 +495,14 @@ func (r *gatewayAPIReconciler) statusUpdateForGateway(gtw *gwapiv1b1.Gateway, sv | |||
} | |||
gCopy := g.DeepCopy() | |||
gCopy.Status.Conditions = status.MergeConditions(gCopy.Status.Conditions, gtw.Status.Conditions...) | |||
for index := range gCopy.Status.Conditions { | |||
gCopy.Status.Conditions[index].ObservedGeneration = gtw.Generation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this loop needs to be removed, this should happen automatically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After removing them, CI fails : (
@@ -894,6 +902,15 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context) { | |||
} | |||
gCopy := g.DeepCopy() | |||
gCopy.Status.Listeners = val.Status.Listeners | |||
|
|||
for index := range gCopy.Status.Conditions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this loop needs to be removed as well
@@ -922,6 +939,12 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context) { | |||
} | |||
hCopy := h.DeepCopy() | |||
hCopy.Status.Parents = val.Status.Parents | |||
|
|||
for parentIndex, parent := range hCopy.Status.Parents { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
@Xunzhuo you will also need to edit this code to
|
@Xunzhuo you will also need to rm gateway/internal/status/status.go Line 164 in 97032be
|
1f4f5d0
to
f6a179e
Compare
Fixes: #839
Fixes: #716
Closes: #828
Signed-off-by: bitliu bitliu@tencent.com