Skip to content
This repository has been archived by the owner on Feb 2, 2021. It is now read-only.

WIP: Replace templating and custom deployment with Helm #224

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Waterdrips
Copy link
Contributor

@Waterdrips Waterdrips commented Oct 15, 2020

This PR replaces the custom OFC installation scripts (shell script
and golang) with the OFC Helm chart which is pulled from the OFC release
specified in the init.yaml.

This PR requires openfaas/openfaas-cloud#680 merged, and ofc-version master selected in ofc-bootstrap, or waiting for a new ofc release

Fixes #223
Fixes #154

Signed-off-by: Alistair Hey alistair@heyal.co.uk

Description

How Has This Been Tested?

This has been tested using my init.yaml taken from a working cluster and
applied to a new cluster (same init.yaml) with the new deployment method
using the chart
This was using Route53 DNS on a Civo k3s cluster

Checklist:

I have:

  • checked my changes follow the style of the existing code / OpenFaaS repos
  • updated the documentation and/or roadmap in README.md
  • read the CONTRIBUTION guide
  • signed-off my commits with git commit -s
  • added unit tests

@Waterdrips Waterdrips force-pushed the waterdrips-helm-chart branch 2 times, most recently from 417795a to 8fc513c Compare October 15, 2020 17:54
@Waterdrips
Copy link
Contributor Author

Waterdrips commented Oct 15, 2020

I want to a few deploys over a few sets of options (customer URL, customers secret, DO DNS, etc) before im willing to remove the WIP status.

I also need to do a full feature-comparison with whats been removed vs added.

@Waterdrips Waterdrips force-pushed the waterdrips-helm-chart branch 6 times, most recently from d9987a4 to 247732a Compare October 16, 2020 20:27
@Waterdrips Waterdrips marked this pull request as ready for review October 16, 2020 20:35
@Waterdrips Waterdrips changed the title WIP: Replace templating and custom deployment with Helm Replace templating and custom deployment with Helm Oct 16, 2020
@Waterdrips Waterdrips force-pushed the waterdrips-helm-chart branch 4 times, most recently from b95ce84 to 8a2b86d Compare November 6, 2020 15:56
@@ -114,7 +114,7 @@ secrets:
value_from: "~/Downloads/service-account.json"
filters:
- "gcp_dns01"
namespace: "cert-manager"
namespace: "openfaas"
Copy link
Member

Choose a reason for hiding this comment

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

Why did all these namespaces change?

Copy link
Member

Choose a reason for hiding this comment

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

I would expect the DNS01 secret to continue to live in the cert-manager namespace?

@@ -105,26 +103,6 @@ func Apply(plan types.Plan) error {
return dashboardConfigErr
}

if plan.EnableOAuth {
Copy link
Member

Choose a reason for hiding this comment

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

How is this done going forward?

@@ -133,12 +111,6 @@ func Apply(plan types.Plan) error {
return stackErr
}

if builderErr := generateTemplate("of-builder-dep", plan, builderConfig{
Copy link
Member

Choose a reason for hiding this comment

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

Certain that this is updated in the new chart?

@@ -2,7 +2,7 @@

rm -rf ./tmp/openfaas-cloud

git clone https://github.com/openfaas/openfaas-cloud ./tmp/openfaas-cloud
git clone https://github.com/openfaas/openfaas-cloud --depth 1 ./tmp/openfaas-cloud
Copy link
Member

Choose a reason for hiding this comment

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

You can checkout a branch directly apparently with --branch

Not sure about SHAs though

Copy link
Member

@alexellis alexellis left a comment

Choose a reason for hiding this comment

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

Given the amount of change in the PR, I have some questions that I would like you to look into before I merge.

@alexellis
Copy link
Member

@Waterdrips please could you see my comments?

@Waterdrips
Copy link
Contributor Author

Yeh there's going to be a load of re work now we have multi arch.

I'll address comments while doing the updates

This commit replaces the custom OFC installation scripts (shell script
and golang) with the OFC Helm chart which is pulled from the OFC release
specified in the init.yaml.

This has been tested using my init.yaml taken from a working cluster and
applied to a new cluster (same init.yaml) with the new deployment method
using the chart

Signed-off-by: Alistair Hey <alistair@heyal.co.uk>
This should allow passing the --update-cloud flag which will
only upgrade/install the OFC core components (chart AND stack.yaml etc)

This also fixes the ClusterIssuer -> Issuer move so the secrets for DNS
need to be in the issuer namespace

Signed-off-by: Alistair Hey <alistair@heyal.co.uk>
@Waterdrips Waterdrips changed the title Replace templating and custom deployment with Helm WIP: Replace templating and custom deployment with Helm Dec 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants