Skip to content

Add new field ImageDigests to Revision status for multi container support#7183

Closed
savitaashture wants to merge 1 commit intoknative:masterfrom
savitaashture:multicontainer-part2
Closed

Add new field ImageDigests to Revision status for multi container support#7183
savitaashture wants to merge 1 commit intoknative:masterfrom
savitaashture:multicontainer-part2

Conversation

@savitaashture
Copy link
Copy Markdown
Contributor

@savitaashture savitaashture commented Mar 7, 2020

Fixes:
Parts of #5822 #3384

Proposed Changes

  • Divided PR to multiple parts in order to cover all the functionality with sub PR's

Part 1: PR

Part 2:

  • Add imageDigests field to revision status in order to maintain digests for all the containers

Release Note

Need to update ImageDigests along with ImageDigest for Revision status

Dependency:
This PR has dependency with PR so once that is merged will update this PR.

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Mar 7, 2020
@knative-prow-robot knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 7, 2020
knative-prow-robot
knative-prow-robot reviewed Mar 7, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@knative-prow-robot knative-prow-robot left a comment

Choose a reason for hiding this comment

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

@savitaashture: 1 warning.

Details

In response to this:

Fixes:
Parts of #5822 #3384

Proposed Changes

  • Divided PR to multiple parts in order to cover all the functionality with sub PR's

Part 1: PR

Part 2:

  • Add imageDigests field to revision status in order to maintain digests for all the containers

Release Note

Need to update ImageDigests along with ImageDigest for Revision status

Dependency:
This PR has dependency with PR so once that is merged will update this 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.

Comment thread pkg/reconciler/revision/revision_test.go Outdated
@knative-prow-robot knative-prow-robot added the area/API API objects and controllers label Mar 7, 2020
@knative-prow-robot
Copy link
Copy Markdown
Contributor

knative-prow-robot commented Mar 7, 2020

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: savitaashture
To complete the pull request process, please assign dprotaso
You can assign the PR to them by writing /assign @dprotaso in a comment when ready.

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

Details Needs approval from an approver in each of these files:

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

@savitaashture savitaashture changed the title Add image digest changes for multi container support Add new field ImageDigests to Revision status for multi container support Mar 7, 2020
savitaashture
savitaashture commented Mar 7, 2020
View reviewed changes
Comment thread pkg/apis/config/defaults.go
defaultValue: false,
}
nc.EnableMultiContainer = strings.EqualFold(data[b.key], "true")

Copy link
Copy Markdown
Contributor Author

@savitaashture savitaashture Mar 7, 2020

Choose a reason for hiding this comment

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

These changes already covered by PR will remove once PR merge

@savitaashture savitaashture force-pushed the multicontainer-part2 branch from 94d4f37 to 158d614 Compare March 7, 2020 20:47
@savitaashture savitaashture force-pushed the multicontainer-part2 branch from 158d614 to 941f68d Compare March 7, 2020 20:54
@knative-metrics-robot
Copy link
Copy Markdown

knative-metrics-robot commented Mar 7, 2020

The following is the coverage report on the affected files.
Say /test pull-knative-serving-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/config/defaults.go 97.1% 97.2% 0.2
pkg/reconciler/revision/revision.go 93.8% 94.7% 1.0

@knative-prow-robot
Copy link
Copy Markdown
Contributor

knative-prow-robot commented Mar 7, 2020

@savitaashture: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-knative-serving-upgrade-tests 941f68d link /test pull-knative-serving-upgrade-tests
pull-knative-serving-integration-tests 941f68d link /test pull-knative-serving-integration-tests

Full PR test history. Your PR dashboard.

Details

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.

@knative-test-reporter-robot
Copy link
Copy Markdown

knative-test-reporter-robot commented Mar 7, 2020

The following jobs failed:

Test name Triggers Retries
pull-knative-serving-integration-tests 0/3

Failed non-flaky tests preventing automatic retry of pull-knative-serving-integration-tests:

test/conformance/api/v1.TestAnnotationPropagation
test/conformance/api/v1alpha1.TestCustomResourcesLimits
test/conformance/api/v1alpha1.TestAnnotationPropagation
test/conformance/api/v1alpha1.TestProjectedConfigMapVolume
test/conformance/api/v1beta1.TestCustomResourcesLimits
test/e2e.TestSubrouteLocalSTS
test/e2e.TestSubrouteLocalSTS/short

@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 14, 2020
@knative-prow-robot
Copy link
Copy Markdown
Contributor

knative-prow-robot commented Mar 14, 2020

@savitaashture: PR needs rebase.

Details

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.

@dprotaso
Copy link
Copy Markdown
Member

dprotaso commented Apr 8, 2020

Is this PR now superseded by #7373 ?

@savitaashture
Copy link
Copy Markdown
Contributor Author

savitaashture commented Apr 9, 2020

Is this PR now superseded by #7373 ?

yes i will close this PR now
Thanks

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

Reviewers

@knative-prow-robot knative-prow-robot knative-prow-robot left review comments

@markusthoemmes markusthoemmes Awaiting requested review from markusthoemmes

@tcnghia tcnghia Awaiting requested review from tcnghia

Assignees

No one assigned

Labels

area/API API objects and controllers cla: yes Indicates the PR's author has signed the CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@savitaashture @knative-prow-robot @knative-metrics-robot @knative-test-reporter-robot @dprotaso @googlebot