Fix for missing pull secrets in auto created namespaces #2245

bvennam · 2019-12-02T20:25:02Z

Proposed Changes

Check for a pull secret named broker-image-pull-secret in default namespace
Copy broker-image-pull-secret into eventing-broker-ingress & eventing-broker-filter namespaces to be used when pulling from private registry.

Release Note

NONE

knative-prow-robot · 2019-12-02T20:25:17Z

Hi @beemarie. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

pkg/reconciler/namespace/namespace.go

lionelvillard · 2019-12-04T02:28:38Z

/ok-to-test

vaikas

Thanks! I know it's still in WIP, but couple of thoughts.

pkg/reconciler/namespace/namespace.go

googlebot · 2019-12-05T02:48:16Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot · 2019-12-05T02:58:26Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

pkg/reconciler/namespace/resources/secret.go

pkg/reconciler/namespace/namespace.go

config/500-controller.yaml

pkg/reconciler/namespace/namespace.go

knative-metrics-robot · 2019-12-09T15:27:33Z

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

File	Old Coverage	New Coverage	Delta
pkg/reconciler/namespace/controller.go	100.0%	94.1%	-5.9
pkg/reconciler/namespace/namespace.go	76.2%	75.0%	-1.2
pkg/utils/secret.go	Do not exist	80.0%

lionelvillard · 2019-12-09T19:17:19Z

/lgtm

vaikas · 2019-12-13T18:23:16Z

/lgtm
/approve

vaikas · 2019-12-13T18:22:37Z

pkg/reconciler/namespace/namespace.go

+		}
+		_, err := utils.CopySecret(r.KubeClientSet.CoreV1(), system.Namespace(), r.brokerPullSecretName, ns.Name, sa.Name)
+		if err != nil {
+			r.Recorder.Event(ns, corev1.EventTypeNormal, secretCopied,


This should probably not be an EventTypeNormal, but let's fix this in a followup PR. I can do this.

knative-prow-robot · 2019-12-13T18:23:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: beemarie, vaikas

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~~ [vaikas]

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

fix for missing pull secrets in auto created namespaces

5b1b585

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Dec 2, 2019

knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 2, 2019

bvennam changed the title ~~fix for missing pull secrets in auto created namespaces~~ WIP: fix for missing pull secrets in auto created namespaces Dec 2, 2019

knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 2, 2019

knative-prow-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 2, 2019

knative-prow-robot requested review from grantr and nachocano December 2, 2019 20:25

lionelvillard reviewed Dec 2, 2019

View reviewed changes

pkg/reconciler/namespace/namespace.go Outdated Show resolved Hide resolved

lionelvillard reviewed Dec 2, 2019

View reviewed changes

pkg/reconciler/namespace/namespace.go Outdated Show resolved Hide resolved

bvennam added 2 commits December 2, 2019 15:54

var name updates

82e0f1c

move env config to controller

2808ea6

knative-prow-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 4, 2019

vaikas reviewed Dec 4, 2019

View reviewed changes

pkg/reconciler/namespace/namespace.go Outdated Show resolved Hide resolved

pkg/reconciler/namespace/namespace.go Outdated Show resolved Hide resolved

pkg/reconciler/namespace/namespace.go Show resolved Hide resolved

update namespace tests as well as move envconfig to controller

8e5c107

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 5, 2019

move copysecret to pkg/utils

ecf8f01

knative-prow-robot added the area/test-and-release Test infrastructure, tests or release label Dec 5, 2019

mattmoor reviewed Dec 5, 2019

View reviewed changes

googlebot added cla: no Indicates the PR's author has not signed the CLA. and removed cla: yes Indicates the PR's author has signed the CLA. labels Dec 5, 2019

bvennam force-pushed the injection_private_registry branch from af63ba1 to ecf8f01 Compare December 5, 2019 02:58

googlebot added cla: yes Indicates the PR's author has signed the CLA. and removed cla: no Indicates the PR's author has not signed the CLA. labels Dec 5, 2019

mattmoor reviewed Dec 5, 2019

View reviewed changes

bvennam added 3 commits December 4, 2019 22:00

update copyright year

bf321d7

testing for copysecret functionality

416dc88

remove comment

bed64f6

mattmoor reviewed Dec 5, 2019

View reviewed changes

bvennam added 2 commits December 5, 2019 15:59

copyright year

d19dca0

comment

2e1020a

lionelvillard reviewed Dec 5, 2019

View reviewed changes

pkg/reconciler/namespace/resources/secret.go Outdated Show resolved Hide resolved

lionelvillard reviewed Dec 5, 2019

View reviewed changes

pkg/reconciler/namespace/namespace.go Show resolved Hide resolved

lionelvillard reviewed Dec 5, 2019

View reviewed changes

pkg/reconciler/namespace/namespace.go Show resolved Hide resolved

lionelvillard reviewed Dec 6, 2019

View reviewed changes

config/500-controller.yaml Outdated Show resolved Hide resolved

remove exit function and update comment

152ae5a

vaikas reviewed Dec 6, 2019

View reviewed changes

pkg/reconciler/namespace/namespace.go Outdated Show resolved Hide resolved

pkg/reconciler/namespace/namespace.go Outdated Show resolved Hide resolved

bvennam added 2 commits December 6, 2019 18:34

update controller

5e09b48

remove secretfound check

0e205f2

bvennam changed the title ~~WIP: fix for missing pull secrets in auto created namespaces~~ Fix for missing pull secrets in auto created namespaces Dec 9, 2019

knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 9, 2019

knative-prow-robot assigned lionelvillard Dec 9, 2019

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 9, 2019

knative-prow-robot assigned vaikas Dec 13, 2019

vaikas reviewed Dec 13, 2019

View reviewed changes

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 13, 2019

knative-prow-robot merged commit e33dba2 into knative:master Dec 13, 2019

vaikas mentioned this pull request Dec 13, 2019

when secret copy fails, emit warning event #2319

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for missing pull secrets in auto created namespaces #2245

Fix for missing pull secrets in auto created namespaces #2245

bvennam commented Dec 2, 2019

knative-prow-robot commented Dec 2, 2019

lionelvillard commented Dec 4, 2019

vaikas left a comment

googlebot commented Dec 5, 2019

googlebot commented Dec 5, 2019

knative-metrics-robot commented Dec 9, 2019

lionelvillard commented Dec 9, 2019

vaikas commented Dec 13, 2019

vaikas Dec 13, 2019

knative-prow-robot commented Dec 13, 2019

Fix for missing pull secrets in auto created namespaces #2245

Fix for missing pull secrets in auto created namespaces #2245

Conversation

bvennam commented Dec 2, 2019

Proposed Changes

knative-prow-robot commented Dec 2, 2019

lionelvillard commented Dec 4, 2019

vaikas left a comment

Choose a reason for hiding this comment

googlebot commented Dec 5, 2019

googlebot commented Dec 5, 2019

knative-metrics-robot commented Dec 9, 2019

lionelvillard commented Dec 9, 2019

vaikas commented Dec 13, 2019

vaikas Dec 13, 2019

Choose a reason for hiding this comment

knative-prow-robot commented Dec 13, 2019