Skip to content
This repository was archived by the owner on Nov 28, 2022. It is now read-only.

[release-v0.23] add patch of missing versions#1389

Merged
openshift-merge-robot merged 8 commits into
openshift:release-v0.23from
devguyio:old-crds-patch
Sep 3, 2021
Merged

[release-v0.23] add patch of missing versions#1389
openshift-merge-robot merged 8 commits into
openshift:release-v0.23from
devguyio:old-crds-patch

Conversation

@devguyio
Copy link
Copy Markdown

@devguyio devguyio commented Sep 2, 2021

Add a patch which adds the dropped versions of all eventing APIs.

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 2, 2021
@devguyio
Copy link
Copy Markdown
Author

devguyio commented Sep 2, 2021

still investigating why it failed but in the mean while I'll retest
/retest

@matzew
Copy link
Copy Markdown
Member

matzew commented Sep 2, 2021

We actually have to apply the patch file. It's not done automatically.

It's only done automatically when the branch is created

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>
@openshift-ci openshift-ci Bot added the area/test-and-release Issues or PRs related to test and release label Sep 2, 2021
@devguyio
Copy link
Copy Markdown
Author

devguyio commented Sep 2, 2021

We actually have to apply the patch file. It's not done automatically.

It's only done automatically when the branch is created

Done

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>
@matzew
Copy link
Copy Markdown
Member

matzew commented Sep 2, 2021

The reconciler tests are PITA... b/c of KO - I can follow up on that

@matzew
Copy link
Copy Markdown
Member

matzew commented Sep 2, 2021

@devguyio please see here this patch for your branch: devguyio#2

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>
@matzew
Copy link
Copy Markdown
Member

matzew commented Sep 2, 2021

@devguyio here is one more issue:
devguyio#3

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>
@matzew
Copy link
Copy Markdown
Member

matzew commented Sep 2, 2021

    resources.go:61: Pod "brokerdlq-kgesnbxt" is invalid: spec.containers[0].image: Required value

@@ -24,7 +24,7 @@ spec:
restartPolicy: "Never"
containers:
- name: eventshub
image: registry.ci.openshift.org/openshift/knative-v0.23.0:knative-eventing-test-eventshub
image: ko://knative.dev/reconciler-test/cmd/eventshub
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ught... that's a revert we don't want ..

following up on that ...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devguyio here we go devguyio#4

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>
@lberk
Copy link
Copy Markdown
Member

lberk commented Sep 2, 2021

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Sep 2, 2021
@matzew
Copy link
Copy Markdown
Member

matzew commented Sep 2, 2021 via email

@matzew
Copy link
Copy Markdown
Member

matzew commented Sep 2, 2021 via email

@openshift-bot
Copy link
Copy Markdown

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@devguyio
Copy link
Copy Markdown
Author

devguyio commented Sep 3, 2021

FAIL: kntest not installed, please clone knative test-infra repo and run `go install ./kntest/cmd/kntest` to install it

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 3, 2021
@@ -62,7 +62,7 @@ func createCertTemplate(name, namespace string, notAfter time.Time) (*x509.Certi
Organization: []string{organization},
CommonName: commonName,
},
SignatureAlgorithm: x509.SHA256WithRSA,
SignatureAlgorithm: x509.PureEd25519,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a regression, won't work on FIPS enabled clusters.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// regular update using the same manager name.
// Note that the APIVersion field is not related to the Subresource field and
// it always corresponds to the version of the main resource.
Subresource string `json:"subresource,omitempty" protobuf:"bytes,8,opt,name=subresource"`
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this revert would be a regression of 69d6d4e (SRVKS-790)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah 😄 just checked the same

Comment on lines -1160 to -1168

// Subresource is the name of the subresource used to update that object, or
// empty string if the object was updated through the main resource. The
// value of this field is used to distinguish between managers, even if they
// share the same name. For example, a status update will be distinct from a
// regular update using the same manager name.
// Note that the APIVersion field is not related to the Subresource field and
// it always corresponds to the version of the main resource.
Subresource string `json:"subresource,omitempty" protobuf:"bytes,8,opt,name=subresource"`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is another patch we would loos - for OCP 4.9

metrics.Record(ctx, requestCountM.M(1))
metrics.RecordBatch(ctx, requestCountM.M(1),
// Convert time.Duration in nanoseconds to milliseconds
responseTimeInMsecM.M(float64(d.Milliseconds())))
return nil
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a regression of #1301

* Rollback certificate algorithm changes (knative#1281) (knative#1283)

Co-authored-by: Markus Thömmes <markusthoemmes@me.com>

* mute noisy metrics

* [SRVKS-790] Patch subresource to unblock webhooks on 4.9 (knative#1361)

Co-authored-by: Markus Thömmes <markusthoemmes@me.com>
Co-authored-by: Stavros Kontopoulos <skontopo@redhat.com>
@matzew
Copy link
Copy Markdown
Member

matzew commented Sep 3, 2021

/retest

Copy link
Copy Markdown
Member

@aliok aliok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Sep 3, 2021
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Sep 3, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aliok, devguyio, lberk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [aliok,devguyio,lberk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release Issues or PRs related to test and release 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