WIP: Mirror alberto pr #2
Conversation
openshift-ci-robot
added
do-not-merge/work-in-progress
|
/test unit |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: If they are not already assigned, you can assign the PR to them by writing 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 |
|
/test unit |
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
|
/test unit |
4 similar comments
|
/test unit |
|
/test unit |
|
/test unit |
|
/test unit |
|
Sometimes when I run the unit tests locally I see: I have suspicion there are no machinesets at all so the machine deployment actually does not lead to creation of machine sets. Still dunno why. |
ingvagabund
force-pushed
the
mirror-alberto-pr
branch
from
483fb01 to
66a29c1
Compare
|
/test unit |
1 similar comment
|
/test unit |
ingvagabund
force-pushed
the
mirror-alberto-pr
branch
from
66a29c1 to
06c5865
Compare
|
/test unit |
ingvagabund
force-pushed
the
mirror-alberto-pr
branch
from
06c5865 to
ade4c11
Compare
|
/test unit |
…h MS is new/old
The CreationTimestamp is not always the referential value for comparision.
The time.Time.Before method does not always return true when it should.
Leading to new and old MSs to be selected in an oposite way.
Using the RevisionVersion for comparision is more precise and it solves
the case where CreationTimestamp fields of both MSs has the same value.
From observation the following machineset CreationTimestamps led to incorrect ordering of MSs:
item[0]: ResourceVersion:"78", CreationTimestamp:v1.Time{Time:time.Time{wall:0x0, ext:63684460660, loc:(*time.Location)(0x2cb5040)}}
item[1]: ResourceVersion:"82", CreationTimestamp:v1.Time{Time:time.Time{wall:0x0, ext:63684460661, loc:(*time.Location)(0x2cb5040)}}
Even though item[1]'s ext field has higher value than item[0]'s ext field,
item[1] was selected by time.Time.Before as old MS instead of new one.
|
/test unit |
3 similar comments
|
/test unit |
|
/test unit |
|
/test unit |
|
/refresh |
|
/test unit |
ingvagabund
force-pushed
the
mirror-alberto-pr
branch
from
ade4c11 to
6867c29
Compare
|
/test all |
|
@ingvagabund: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
|
closing in favour of #7 |
* Introduce CEL for ClusterClass Variables Signed-off-by: chaunceyjiang <[email protected]> * feat: Implement CEL validation * refactor: Add comments from previous code reviews * chore: Generate CC manifest after fixing list type annotation (openshift#2) * chore: Fix up CRD manifest * fix: Pass through context to CEL funcs * feat: Add CEL admission cost validation * refactor: Add nolint to unbounded * refactor: Fix up new func signature * build: Fix up go mod for tools * fixup! refactor: Apply review feedback * fixup! build: Regenerate openapi spec * fixup! refactor: Apply review feedback * fixup! fix: Regenerate everything * fixup! fix: Apply review feedback * fixup! fix: More review feedback * fixup! refactor: Address review feedback, especially re recursion * fixup! fix: Check total cost * fixup! refactor: Address review feedback - rename testCtx to ctx * CEL: Various improvements (openshift#3) * resolve compile issue after rebase * Some more improvements (openshift#4) --------- Signed-off-by: chaunceyjiang <[email protected]> Co-authored-by: Jimmi Dyson <[email protected]> Co-authored-by: Jimmi Dyson <[email protected]> Co-authored-by: Stefan Büringer <[email protected]> Co-authored-by: Stefan Bueringer <[email protected]>
DO NOT MERGED, just for debugging purposes