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.

Sign up for GitHub

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

Jump to bottom

kn v0.16.0 release prep #922

Merged
merged 11 commits into from
Jul 10, 2020
Merged

kn v0.16.0 release prep #922

merged 11 commits into from
Jul 10, 2020

Conversation

navidshaikh
Copy link
Collaborator

@navidshaikh navidshaikh commented Jul 8, 2020 •
edited
Loading

Description

kn v0.16.0 release prep

Changes

  • Vendor v0.16.0 eventing and serving

  • Update unit test for invalid concurrency target value

  • Commit the LICENSES generated by hack/update-deps.sh

  • Spare third_party dir from license check in hack/build.sh

  • Update eventing and serving version for 'kn version'

  • Use DeprecatedInjectionAnnotation key for backward compatibility

    Fixes Switch to DeprecatedInjectionAnnotation from InjectionAnnotation for backward compatibility #918
    Use DeprecatedInjectionAnnotation, i.e. "knative-eventing-injection"
    for reconciling broker via namespace labeling and trigger annotations.

    From eventing v0.16.0 release, this key is changed to "eventing.knative.dev/injection"
    however, older eventing installation wont recognize it. So we continue
    using the deprecated key until the sugar-controller would support reconciling
    on either keys.

  • Fix(kn version): Display eventing.knative.dev at v1beta1 version

    We've been showing 'v1alpha1' in kn version while we actually
    vendor and refer v1beta1 for eventing.knative.dev

  • chore(e2e): Run tests against serving/eventing v0.16.0 release

  • Update test for concurrency-target to expect new error message

    the new error message for incorrect concurrency-target value contains "should be at least 0.01"

@navidshaikh
Update unit test for invalid concurrency target value
b15113f
@navidshaikh
Vendor v0.16.0 eventing and serving
dd04def
@navidshaikh
Commit the LICENSES generated by hack/update-deps.sh
448061e
@navidshaikh
Spare third_party dir from license check in hack/build.sh
9ff0faa
@navidshaikh
Update eventing and serving version for 'kn version'
d2b117c
@navidshaikh
Use DeprecatedInjectionAnnotation key for backward compatibility
bd32478 
  Fixes knative#918
  Use DeprecatedInjectionAnnotation, i.e. "knative-eventing-injection"
  for reconciling broker via namespace labeling and trigger annotations.

  From eventing v0.16.0 release, this key is changed to "eventing.knative.dev/injection"
  however, older eventing installation wont recognize it. So we continue
  using the deprecated key until the sugar-controller would support reconciling
  on either keys.
@navidshaikh
Fix(kn version): Display eventing.knative.dev at v1beta1 version
a97379a 
 We've been showing 'v1alpha1' in kn version while we actually
 vendor and refer v1beta1 for eventing.knative.dev
@navidshaikh
chore(e2e): Run tests against serving/eventing v0.16.0 release
e2dece1
@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 8, 2020
@knative-prow-robot knative-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 8, 2020
rhuss
rhuss reviewed Jul 8, 2020
View reviewed changes
third_party/VENDOR-LICENSE/github.com/hashicorp/golang-lru/arc.go
@@ -0,0 +1,257 @@
package lru
Copy link
Contributor

Choose a reason for hiding this comment

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

this file looks bogus

Copy link
Contributor

Choose a reason for hiding this comment

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

As it turns out, according to google/go-licenses#28 its because that for "Reciprocal" licenses the whole source code is copied (regardless whether it has been changed or not).

rhuss
rhuss reviewed Jul 8, 2020
View reviewed changes
third_party/VENDOR-LICENSE/github.com/hashicorp/golang-lru/simplelru/lru.go
@@ -0,0 +1,177 @@
package simplelru
Copy link
Contributor

Choose a reason for hiding this comment

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

same ... I don't think the mechanism of picking up those licenses is bullet proof

@navidshaikh
Update test for concurrency-target to expect new error message
448d481 
 the new error message for incorrect concurrency-target value contains "should be at least 0.01"
@rhuss
Copy link
Contributor

rhuss commented Jul 9, 2020

/retest

@rhuss
Copy link
Contributor

rhuss commented Jul 9, 2020

@navid I think we have a real error in the traffic split tests. I.e. that some condition doesn't eventually happens.

@navidshaikh
Copy link
Collaborator Author

@rhuss : broker injection ones instead, while the service options one is flake (ReconcileIngressFailed: Ingress reconciliation failed)

--- FAIL: TestBrokerTrigger (308.78s)
--- FAIL: TestSink (307.00s)
--- FAIL: TestServiceOptions (332.53s)

This is failing against v0.16.0 install of eventing,I will check further.

@navidshaikh
Setup sugar-controller for released eventing version setup
8fe2114 
 - reuse knative_setup util from common lib for latest-release CI setup
maximilien
maximilien approved these changes Jul 9, 2020
View reviewed changes
Copy link
Contributor

@maximilien maximilien left a comment

Choose a reason for hiding this comment

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

Thanks for including licenses in one place. LGTM

@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maximilien, navidshaikh

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:
  • OWNERS [maximilien,navidshaikh]

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

@rhuss
Copy link
Contributor

rhuss commented Jul 10, 2020

Thanks !

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 10, 2020
@knative-prow-robot knative-prow-robot merged commit f91fb57 into knative:master Jul 10, 2020
@navidshaikh navidshaikh deleted the pr/v0.16-release branch July 12, 2020 10:38
@navidshaikh navidshaikh added this to the v0.16.0 milestone Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rhuss rhuss rhuss left review comments

@mattmoor mattmoor mattmoor left review comments

@maximilien maximilien maximilien approved these changes

@evankanderson evankanderson Awaiting requested review from evankanderson

Assignees

@rhuss rhuss

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v0.16.0
Development

Successfully merging this pull request may close these issues.

Switch to DeprecatedInjectionAnnotation from InjectionAnnotation for backward compatibility
5 participants
@navidshaikh @rhuss @knative-prow-robot @maximilien @mattmoor