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

Add Epinio Helm Chart To Rancher Marketplace #1814

Merged
merged 2 commits into from
Sep 29, 2022
Merged

Add Epinio Helm Chart To Rancher Marketplace #1814

merged 2 commits into from
Sep 29, 2022

Conversation

andreas-kupries
Copy link
Contributor

@andreas-kupries andreas-kupries commented Mar 18, 2022 •
edited
Loading

@andreas-kupries andreas-kupries mentioned this pull request Mar 18, 2022
Closed
@andreas-kupries andreas-kupries force-pushed the epinio-1248-add-to-marketplace branch 3 times, most recently from 630c918 to 4e99794 Compare March 28, 2022 11:55
@andreas-kupries andreas-kupries force-pushed the epinio-1248-add-to-marketplace branch 3 times, most recently from da219d6 to 95b559d Compare March 29, 2022 13:12
@pennyscissors pennyscissors self-requested a review April 1, 2022 17:54
@andreas-kupries andreas-kupries force-pushed the epinio-1248-add-to-marketplace branch 2 times, most recently from 6206045 to 19026e0 Compare April 5, 2022 11:13
@andreas-kupries andreas-kupries marked this pull request as ready for review April 14, 2022 14:55
@andreas-kupries andreas-kupries marked this pull request as draft April 14, 2022 14:55
@andreas-kupries
Copy link
Contributor Author

marked ready mistake. reverted.

@andreas-kupries andreas-kupries force-pushed the epinio-1248-add-to-marketplace branch 2 times, most recently from 18fa8f7 to 7c5c008 Compare April 29, 2022 11:25
@andreas-kupries andreas-kupries mentioned this pull request Apr 29, 2022
Merged
8 tasks
@andreas-kupries andreas-kupries force-pushed the epinio-1248-add-to-marketplace branch from 7a8304c to 937aaa1 Compare May 9, 2022 14:13
pennyscissors
pennyscissors reviewed May 23, 2022
View reviewed changes
Copy link
Contributor

@pennyscissors pennyscissors left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Left some comments for annotations. Also, looks like this PR needs to be rebased due to conflicts.

@pennyscissors pennyscissors requested a review from aiyengar2 May 23, 2022 23:48
@andreas-kupries andreas-kupries force-pushed the epinio-1248-add-to-marketplace branch from 937aaa1 to 1a90094 Compare May 24, 2022 10:49
@andreas-kupries andreas-kupries force-pushed the epinio-1248-add-to-marketplace branch from 1a90094 to 4cd4e02 Compare May 25, 2022 13:55
@andreas-kupries andreas-kupries marked this pull request as ready for review May 25, 2022 13:56
pennyscissors
pennyscissors previously approved these changes May 26, 2022
View reviewed changes
Copy link
Contributor

@pennyscissors pennyscissors left a comment

Choose a reason for hiding this comment

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

LGTM. Should be good to merge after @aiyengar2 approves and rancher/image-mirror PR is merged.

aiyengar2
aiyengar2 suggested changes May 31, 2022
View reviewed changes
@jimmykarily
Copy link

jimmykarily commented Aug 11, 2022 •
edited
Loading

I think we should stub the paketo builder version to the one we mirrored. Right now the tag is just full.

@jimmykarily
Copy link

I think we should stub the paketo builder version to the one we mirrored. Right now the tag is just full.

Fixed here: d4a71b4

@jimmykarily jimmykarily force-pushed the epinio-1248-add-to-marketplace branch from f442995 to f3a472c Compare August 11, 2022 12:22
@andreas-kupries andreas-kupries force-pushed the epinio-1248-add-to-marketplace branch from f3a472c to a9ad49a Compare September 8, 2022 12:24
@andreas-kupries andreas-kupries mentioned this pull request Sep 8, 2022
Merged
8 tasks
@andreas-kupries
Copy link
Contributor Author

v1.2.0 images - rancher/image-mirror#283

@andreas-kupries andreas-kupries force-pushed the epinio-1248-add-to-marketplace branch from 2f52274 to c10fb37 Compare September 8, 2022 14:58
This was referenced Sep 9, 2022
Merged
Merged
@andreas-kupries andreas-kupries force-pushed the epinio-1248-add-to-marketplace branch 2 times, most recently from 0d72ae6 to 1f9174d Compare September 13, 2022 16:34
@andreas-kupries
Copy link
Contributor Author

andreas-kupries commented Sep 13, 2022 •
edited
Loading

I am not sure what is going on - commit 1f9174d contains the changes made to satisfy the local make validate. Now it claims to be in conflict ?

Please provide guidance to resolve this.

@andreas-kupries andreas-kupries force-pushed the epinio-1248-add-to-marketplace branch 2 times, most recently from dcd433d to 381a033 Compare September 22, 2022 09:14
pennyscissors
pennyscissors suggested changes Sep 27, 2022
View reviewed changes
Copy link
Contributor

@pennyscissors pennyscissors left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, just a few comments and also the commits need to be squashed. Can you squash them into two commits? First should be all the changes you made to the package, and second should be the output of running make charts like what you have as your last commit currently (we prefer to keep this format for chart PRs).

@andreas-kupries
epinio package and chart. Sub charts (minio, kubed, epinio-ui) as dep…
48ff98d 
…endent packages.

See directory `package/epinio/epinio`, and siblings.

See issue epinio/epinio#1248
Based on upstream chart 1.2.1
Chart patches:
- tolerations, node selectors
- mirrored images, added registry to values.yaml where needed
- SVG icon for assets
- cattle annotations, versioning
- validate presence of cert manager crds
- dropped standard/default application namespace
- psp and associated resources
- standard image for kubectl
- not for shell, breaks staging - bci-micro based epinio-unpacker
- standard appchart provision as chart asset file instead of external ref
@andreas-kupries
make chart
891495b
@andreas-kupries andreas-kupries force-pushed the epinio-1248-add-to-marketplace branch from 381a033 to 891495b Compare September 28, 2022 10:49
@andreas-kupries
Copy link
Contributor Author

History squashed, make chart last & separate, as before.
Commits 48ff98d, 891495b

@pennyscissors pennyscissors self-requested a review September 28, 2022 17:51
@thehejik
Copy link

QA report

Testing environment

Rancher v2.6.9-rc2 on top of a single-node k3s v1.22.7 cluster with incorporated traefik lb. Helm chart git repository for Epinio Rancher App installation https://github.com/epinio/charts.git and branch epinio-1248-add-to-marketplace

Tested

  • Adding the custom repository for this PR
  • Installation of Epinio App (100.0.0+up1.2.1) helm chart in Rancher (local k3s cluster) by using default values (+entered base epinio domain) - epinio and epinio-crd apps were successfully deployed in cattle-epinio-system namespace
  • All the input boxed for values were ok
  • All the relevant resources like pods are in Ready state
  • Accessed epinio-ui over ingress, login by using default credentials
  • Created epinio namespace for its applications
  • Deployment of sample-app phpinfo() app stored in local folder
  • Accessing the sample-app over provided URL in epinio-ui
  • All resources still ok

Everything was working as expected.

Created epinio sample-app detail:
epinio-rancher

@thehejik
Copy link

QA report for Air-Gap environment

Testing environment

AG Cluster and Bastion deployed over Rancher jenkins CI job in AWS. Rancher v2.6.9-rc1 on top of a 3-node HA RKE v1.3.15 k8s v1.24.4 cluster with incorporated nginx-ingress lb. All nodes are running default Ubuntu.

In addition to self hosted registries on Bastion I had to clone also tgz helm archives (for epinio-crd, epinio and cert-manager) to custom ChartMuseum server running on the Bastion - then I've added the helm chart repository to Rancher as HTTP repo in form http://museum.my.bastion.private.ip.nip.io:8080.

Other requirements:

  • cert-manager incl. its CRD
  • local-path provisioner (not available by default in RKE) + setting as default SC

Tested

  • Adding the custom repository by using chartmuseum
  • Installation of Epinio App (100.0.0+up1.2.1) helm chart in Rancher (local cluster) by using default values (+entered base epinio domain) - epinio and epinio-crd apps were successfully deployed in cattle-epinio-system namespace
  • All the input boxed for values were ok
  • All the relevant resources like pods are in Ready state
  • Accessed epinio-ui over ingress, login by using default credentials
  • Created epinio namespace for its applications
  • Deployment of sample-app phpinfo() app stored in local folder
  • Accessing the sample-app over provided URL in epinio-ui
  • All resources still ok
  • Uninstalled ok (delete epinio and then epinio-crd App)

Everything was working as expected.

This is list of images related to Epinio (100.0.0+up1.2.1) I had to push to my registries:

rancher/mirrored-epinio-epinio-server:v1.2.0
rancher/mirrored-epinio-epinio-ui:v1.2.0-0.0.1
rancher/mirrored-appscode-kubed:v0.13.2
rancher/mirrored-minio-minio:RELEASE.2022-05-08T23-50-31Z
rancher/mirrored-minio-mirrored-minio-mc:RELEASE.2022-05-09T04-08-26Z
rancher/mirrored-library-nginx:1.23.0-alpine
rancher/mirrored-paketobuildpacks-builder:0.2.95-full
rancher/mirrored-library-registry:2.8.1
rancher/mirrored-epinio-epinio-unpacker:1.0
rancher/mirrored-amazon-aws-cli:2.0.52

Deployed epinio with sample application:
epinio-rancher-ag

thehejik
thehejik approved these changes Sep 29, 2022
View reviewed changes
Copy link

@thehejik thehejik left a comment

Choose a reason for hiding this comment

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

I approve for Epinio QA side. It was tested on a single-node k3s cluster as well as on a 3-node HA RKE cluster running in AirGap environment. All good.

@pennyscissors pennyscissors merged commit 49128f0 into rancher:dev-v2.6 Sep 29, 2022
@pennyscissors pennyscissors mentioned this pull request Sep 29, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimmykarily jimmykarily jimmykarily left review comments

@juadk juadk juadk left review comments

@mmartin24 mmartin24 mmartin24 left review comments

@pennyscissors pennyscissors pennyscissors approved these changes

@thehejik thehejik thehejik approved these changes

@aiyengar2 aiyengar2 Awaiting requested review from aiyengar2

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@andreas-kupries @jimmykarily @thehejik @juadk @pennyscissors @aiyengar2 @mmartin24 @cmurphy @rmweir @Jono-SUSE-Rancher