Add portus chart #2766

rendhalver · 2017-11-16T19:28:24Z

This is a resubmit of #1284.
Thanks to @so0k for all the hard work!
This is currently the same as the last commit in that PR.

It looks like there are a few differences in layout since that PR was submitted so I will try to address those concerns as I go.
There were also a few concerns not address in the original PR so I will address those as well.

unguiculus · 2017-11-16T20:34:20Z

Please check our best practices, especially those on naming resources and standard labels. Resource metadata should generally look like so:

name: {{ template "portus.fullname" . }}
labels:
  heritage: {{ .Release.Service }}
  release: {{ .Release.Name }}
  chart: {{ .Chart.Name }}-{{ .Chart.Version }}
  app: {{ template "portus.name" . }}

rendhalver · 2017-11-16T20:40:28Z

Thanks @unguiculus I will get those fixed.

rendhalver · 2017-11-17T15:19:21Z

@unguiculus This Chart has multiple components but there isn't a definition $component.name in the _helpers.tpl file and I am not sure of the proper way to do that.
Can you point me to some docs on how to achieve that?

On a side note I am very tempted to split out the registry component into it's own chart and then add that as a requirement for this chart so the templates are a bit simpler.

I am also new to writing Charts so I am still learning how best to do them.

unguiculus · 2017-11-17T16:03:45Z

There's a chart for Docker registry already: https://github.com/kubernetes/charts/tree/master/incubator/docker-registry

rendhalver · 2017-11-17T16:23:32Z

True. I forgot about that Chart.
I had already discounted that because doesn't have the options we need to setup the registry for Portus.
Maybe I can expand that one so it can be configured with those extra options.

As for my other question what is the best way to define those $component.name variables?

rendhalver · 2017-11-17T17:15:49Z

@unguiculus I think worked out what I needed to change.
The latest commit addresses the label inconsistencies.

unguiculus · 2017-11-17T18:30:13Z

Please create a PR for the registry chart to add the stuff you need. We should also see that we can move it to stable so you can use it.

rendhalver · 2017-11-17T18:31:52Z

Will do.
Thanks for the help @unguiculus It is much appreciated. :)

so0k · 2017-11-20T08:37:50Z

The reason I had them bundled was the auth token service configuration required on the registry side.

If the registry chart exposes that, it should be fine to define it as a dependency though

rendhalver · 2017-11-20T21:12:09Z

@unguiculus What else needs changing in the docker-registry chart to move it to stable?

viglesiasce · 2017-11-27T16:51:12Z

/ok-to-test

rendhalver · 2017-11-27T17:25:40Z

@viglesiasce This is dependent on my other PR for getting docker-registry usable and put of incubator #2801. Once that is sorted I can update this to reflect the new location of docker-registry.

unguiculus · 2017-11-29T20:04:49Z

I think the Docker registry chart looks pretty good. I'd suggest you create a PR to move it to stable.

unguiculus · 2017-11-22T20:32:42Z

stable/portus/Chart.yaml

+apiVersion: v1
+name: portus
+home: http://port.us.org/
+version: 0.1.0


Add appVersion

unguiculus · 2017-11-22T20:32:52Z

stable/portus/Chart.yaml

+  - https://github.com/docker/distribution/
+  - https://github.com/Nordstrom/kube-registry
+maintainers:
+  - name: Vincent De Smet


Use Github username

can remove me if this is holding back the merge :)

rendhalver · 2017-11-29T20:10:03Z

It needs to be redone or rebased. Redoing it seems quicker at this stage. #2612 got merged that changed a lot of the docker-registry chart.
I will get that sorted when I get a chance.

rendhalver · 2017-12-04T16:08:54Z

The docker-registry PR has been redone in #2926.

rendhalver · 2017-12-06T19:31:36Z

@unguiculus This is my PR to promote docker-registry to stable: #2944
I can update the requirements on this once that is merged.

rendhalver · 2017-12-15T12:46:51Z

/retest

unguiculus · 2017-12-18T19:02:12Z

Let's try it again. Timeouts have been increased.

rendhalver · 2018-08-14T19:03:36Z

Thanks I will get that fixed.

rendhalver · 2018-08-14T19:17:12Z

Why didn't that failure fail the build?

davidkarlsen · 2018-08-14T20:50:57Z

@rendhalver Have a look at helm/chart-testing#9

unguiculus · 2018-08-15T06:14:07Z

/retest

unguiculus · 2018-08-15T06:21:21Z

/test pull-charts-e2e

rendhalver · 2018-08-15T12:26:06Z

I see what's going on.
It's the problem we had with testing before where the deploy is assuming the Secret exists before the chart it setup.
I will put in the code to create those certs if they don't exist.

unguiculus · 2018-08-15T12:54:13Z

stable/portus/templates/portus-post-delete-job.yaml

+    metadata:
+      name: "{{ .Release.Name }}"
+      labels:
+        app: {{ template "portus.name" . }}


Add release label to selector.

Thanks for the reminder.
I will get that fixed.

unguiculus · 2018-08-15T12:55:51Z

stable/portus/templates/minio-create-bucket-job.yaml

+      name: "{{ .Release.Name }}"
+      labels:
+        app: "{{ template "portus.name" . }}"
+        component: minio


Add release label to selector.

Thanks for the reminder.
I will get that fixed.

Previosly we assumed the cert for the secrets existed. This moves the creation of that secret into the chart. If no certs are provided we generate them

k8s-ci-robot · 2018-08-16T19:23:07Z

@rendhalver: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-charts-e2e	`f7999bb`	link	`/test pull-charts-e2e`

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.

unguiculus · 2018-08-16T19:25:31Z

With the latest chart-testing fix the CI job finally fails as expected.

stale · 2018-09-15T20:01:03Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

giacomoguiulfo · 2018-09-15T21:18:24Z

stable/portus/.helmignore

+# Various IDEs
+.project
+.idea/
+*.tmproj


Please add the OWNERS file in the .helmignore - https://github.com/helm/charts#owning-and-maintaining-a-chart

stale · 2018-10-15T21:45:47Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

stale · 2018-10-29T22:28:31Z

This issue is being automatically closed due to inactivity.

flavio · 2018-10-30T15:50:21Z

Triggering the reopening of this issue. @mssola is going to take care of that unless @rendhalver or others have something against it.

rendhalver · 2018-10-31T12:00:25Z

I am happy to help get it working. I haven't had the time to devote to it lately.
I am also happy to be a maintainer and OWNER if you need one.

rendhalver · 2018-10-31T12:01:17Z

And thanks. :)
I had a few ideas on how to get it working but didn't get time top test them.
Ping my on your new PR and I will add those thoughts there.

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 16, 2017

unguiculus added the changes needed label Nov 16, 2017

unguiculus self-assigned this Nov 16, 2017

rendhalver mentioned this pull request Nov 20, 2017

Add extra configuration options to docker-registry #2801

Closed

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 24, 2017

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 27, 2017

unguiculus suggested changes Nov 29, 2017

View reviewed changes

rendhalver mentioned this pull request Dec 4, 2017

[incubator/docker-registry] Add extra configuration options for docker-registry #2926

Merged

rendhalver force-pushed the feature/portus branch from 106bc89 to 0a637c4 Compare December 13, 2017 18:55

k8s-ci-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 14, 2018

Fix volumeMounts for portus background container

06bb523

unguiculus suggested changes Aug 15, 2018

View reviewed changes

Add release to selector labels

ae9c928

unguiculus mentioned this pull request Aug 15, 2018

Double-check pod readiness for deployments helm/chart-testing#10

Merged

Pete Brown and others added 3 commits August 15, 2018 11:08

Generate TLS certs if not provided

8ccaa6a

Previosly we assumed the cert for the secrets existed. This moves the creation of that secret into the chart. If no certs are provided we generate them

Remove log_level env var from background container

4f626d7

Merge remote-tracking branch 'upstream/master' into HEAD

f7999bb

mattfarina added the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Aug 27, 2018

stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 15, 2018

giacomoguiulfo reviewed Sep 15, 2018

View reviewed changes

stale bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 15, 2018

mssola mentioned this pull request Sep 20, 2018

Another Authorization token failure for backround.rb SUSE/Portus#1624

Open

stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 15, 2018

stale bot closed this Oct 29, 2018

bklang deleted the feature/portus branch December 30, 2018 20:14

Add portus chart #2766

Add portus chart #2766

Conversation

rendhalver commented Nov 16, 2017

unguiculus commented Nov 16, 2017

rendhalver commented Nov 16, 2017

rendhalver commented Nov 17, 2017

unguiculus commented Nov 17, 2017

rendhalver commented Nov 17, 2017

rendhalver commented Nov 17, 2017

unguiculus commented Nov 17, 2017

rendhalver commented Nov 17, 2017

so0k commented Nov 20, 2017

rendhalver commented Nov 20, 2017

viglesiasce commented Nov 27, 2017

rendhalver commented Nov 27, 2017

unguiculus commented Nov 29, 2017 • edited Loading

unguiculus Nov 22, 2017

Choose a reason for hiding this comment

unguiculus Nov 22, 2017

Choose a reason for hiding this comment

so0k Dec 15, 2017

Choose a reason for hiding this comment

rendhalver commented Nov 29, 2017

rendhalver commented Dec 4, 2017

rendhalver commented Dec 6, 2017

rendhalver commented Dec 15, 2017

unguiculus commented Dec 18, 2017

rendhalver commented Aug 14, 2018

rendhalver commented Aug 14, 2018

davidkarlsen commented Aug 14, 2018

unguiculus commented Aug 15, 2018

unguiculus commented Aug 15, 2018

rendhalver commented Aug 15, 2018

unguiculus Aug 15, 2018

Choose a reason for hiding this comment

rendhalver Aug 15, 2018

Choose a reason for hiding this comment

unguiculus Aug 15, 2018

Choose a reason for hiding this comment

rendhalver Aug 15, 2018

Choose a reason for hiding this comment

k8s-ci-robot commented Aug 16, 2018

unguiculus commented Aug 16, 2018

stale bot commented Sep 15, 2018

giacomoguiulfo Sep 15, 2018 • edited Loading

Choose a reason for hiding this comment

stale bot commented Oct 15, 2018

stale bot commented Oct 29, 2018

flavio commented Oct 30, 2018

rendhalver commented Oct 31, 2018

rendhalver commented Oct 31, 2018

unguiculus commented Nov 29, 2017 •

edited

Loading

giacomoguiulfo Sep 15, 2018 •

edited

Loading