Add bootstrap CLI controllers configurations #3647

waleedhammam · 2023-11-19T15:18:29Z

What changed?

Add PolicyAgent, TFController Controller as steps

How did you validate the change?

Explain how a reviewer can verify the change themselves

a) using non-interactive

1- install with one controller

gitops bootstrap --components-extra=policy-agent

2- install with multiple controllers

gitops bootstrap --components-extra="policy-agent,tf-controller"

b) interactive session

run gitops bootstrap and follow on screen instructions

c) silent mode: gitops bootstrap -s

Integration tests -- what is covered, what cannot be covered;
or, explain why there are no new tests
Unit tests -- what is covered, what cannot be covered; are
there tests that fail without the change?

Release notes

Documentation Changes

Other follow ups

enekofb

The scope of this review covers the general code aspects of the PR. Raising it now so you could start having a look to the suggestions.

Next review will cover the extra controllers logic and one of the controllers.

cmd/gitops/app/bootstrap/cmd.go

pkg/bootstrap/bootstrap.go

cmd/gitops/app/bootstrap/cmd_acceptance_test.go

cmd/gitops/app/bootstrap/cmd.go

cmd/gitops/app/bootstrap/cmd_acceptance_test.go

pkg/bootstrap/steps/ask_bootstrap_flux.go

pkg/bootstrap/steps/step.go

pkg/bootstrap/steps/oidc.go

enekofb

this review includes extra controllers logic and policy agent controller (as example of controller, my comments for policy-agent applies to the other controllers too)

pkg/bootstrap/steps/extra_controllers_test.go

pkg/bootstrap/steps/extra_controllers.go

pkg/bootstrap/steps/policy_agent.go

enekofb

Follow-ups / suggestions:

Log known-issue for cert-manager in release notes and documentation
handle cert-manager dependency
to unify install controller logic into a single one with configuration if they are common
(general) to have a better approach for cmd Examples: orient only to journeys, currently stands a bit weird as configuration options.

enekofb · 2023-12-06T17:32:26Z

pkg/bootstrap/steps/ask_bootstrap_flux.go

@@ -30,12 +31,16 @@ func askBootstrapFlux(input []StepInput, c *Config) ([]StepOutput, error) {
 	if !canAskForFluxBootstrap(input, c) {
 		return []StepOutput{}, nil
 	}
+	if c.BootstrapFlux && c.Silent {
+		c.Logger.Generatef("bootstrapping flux in the generic way")


this message might be misleading as you are generating output ... it feels a better place where the action really happens

enekofb · 2023-12-06T17:38:49Z

pkg/bootstrap/steps/components_extra.go

+		Name:         inComponentsExtra,
+		Type:         multiSelectionChoice,
+		Msg:          componentsExtraMsg,
+		Values:       ComponentsExtra,


here you should not be able to select all values but the one that you could install which are the total components - the ones that you have installed ... otherwise you could select to install tf-controller when it is already installed.

enekofb · 2023-12-06T17:40:31Z

pkg/bootstrap/steps/components_extra.go

+				return []StepOutput{}, fmt.Errorf("can't install tf controller: %v", err)
+			}
+		case "":
+			c.Logger.Successf("selected none extra components")


Suggested change

c.Logger.Successf("selected none extra components")

.. given there is nothing to do

enekofb · 2023-12-06T17:42:24Z

pkg/bootstrap/steps/config.go

@@ -204,6 +222,11 @@ func (cb *ConfigBuilder) Build() (Config, error) {
 		return Config{}, fmt.Errorf("error creating git repository configuration: %v", err)
 	}

+	componentsExtraConfig, err := NewInstallExtraComponentsConfig(cb.componentsExtra, kubeHttp.Client)


enekofb · 2023-12-06T17:43:29Z

pkg/bootstrap/steps/policy_agent.go

+	policyAgentInstallConfirmMsg = "Policy Agent is installed successfully"
+)
+
+const (


enekofb · 2023-12-06T17:46:10Z

pkg/bootstrap/steps/step.go

@@ -147,9 +147,15 @@ func defaultInputStep(inputs []StepInput, c *Config, stdin io.ReadCloser) ([]Ste
 			if input.StepInformation != "" {
 				c.Logger.Warningf(input.StepInformation)
 			}
-			// if silent mode is enabled, select yes
+			// if silent mode is enabled, select the default value
+			// if no default value select yes by default


Suggested change

// if no default value select yes by default

// if no default value an error will be returned

enekofb · 2023-12-07T08:28:48Z

cmd/gitops/app/bootstrap/cmd_acceptance_test.go

 				"--password=admin123",
 				"--discovery-url=https://dex-01.wge.dev.weave.works/.well-known/openid-configuration",
 				"--client-id=weave-gitops-enterprise",
 				gitUsernameFlag, gitPasswordFlag, gitBranchFlag, gitRepoPathFlag,
 				repoHTTPSURLFlag,
 				oidcClientSecretFlag, "-s",
+				"--components-extra=policy-agent,tf-controller",


Tested that acceptance test passed

◎ reconciling changes === RUN TestBootstrapCmd/journey_flux_does_not_exist:_should_bootstrap_with_valid_arguments ✔ changes are reconciled successfully! ◎ install extra components ⚠️ please note that the Policy Agent requires cert-manager to be installed! ► installing Policy Agent ... ✔ Policy Agent is installed successfully ► write file to repo: policy-agent-helmrelease.yaml ► cloning flux git repo: flux-system/flux-system ✔ cloned flux git repo: flux-system/flux-system ✔ file committed to repo: policy-agent-helmrelease.yaml ◎ reconciling changes ✔ changes are reconciled successfully! ► installing Terraform Controller ... ► rendered WGE HelmRelease file ► updating HelmRelease file ✔ Terraform Controller is installed successfully ► write file to repo: tf-controller.yaml ► cloning flux git repo: flux-system/flux-system ✔ cloned flux git repo: flux-system/flux-system ✔ file committed to repo: tf-controller.yaml ◎ reconciling changes ✔ changes are reconciled successfully! ► write file to repo: wge-hrelease.yaml ► cloning flux git repo: flux-system/flux-system ✔ cloned flux git repo: flux-system/flux-system ✔ file committed to repo: wge-hrelease.yaml ◎ reconciling changes ✔ changes are reconciled successfully! ◎ preparing dashboard domain ✔ WGE v0.37.0 is installed successfully. To access the dashboard, run the following command to create portforward to the dasboard local domain http://localhost:8000 ► credentials for accessing the admin dashboard username: `wego-admin` kubectl -n flux-system port-forward svc/clusters-service 8000:8000 cmd_acceptance_test.go:291: "level"=0 "msg"="deleted object" "name"="weave-gitops-enterprise-credentials" "ns"="flux-system" "kind"="Secret" cmd_acceptance_test.go:291: "level"=0 "msg"="deleted object" "name"="cluster-user-auth" "ns"="flux-system" "kind"="Secret" --- PASS: TestBootstrapCmd (130.15s) --- PASS: TestBootstrapCmd/journey_flux_does_not_exist:_should_bootstrap_with_valid_arguments (130.14s)

enekofb · 2023-12-07T08:33:28Z

cmd/gitops/app/bootstrap/cmd_acceptance_test.go

 				"--password=admin123",
 				"--discovery-url=https://dex-01.wge.dev.weave.works/.well-known/openid-configuration",
 				"--client-id=weave-gitops-enterprise",
 				gitUsernameFlag, gitPasswordFlag, gitBranchFlag, gitRepoPathFlag,
 				repoHTTPSURLFlag,
 				oidcClientSecretFlag, "-s",
+				"--components-extra=policy-agent,tf-controller",
+				"--bootstrap-flux",


i could see the following pods

➜ weave-gitops-enterprise git:(3276-cli-controllers-eneko) ✗ k get pods <aws:sts> NAME READY STATUS RESTARTS AGE cluster-bootstrap-controller-manager-79cbb5dfbc-dz5rd 2/2 Running 0 76s cluster-controller-manager-86c496b9f7-5tk4k 2/2 Running 0 76s gitopssets-controller-manager-5cc97766b9-knvb4 2/2 Running 0 76s helm-controller-b957fcf89-9ptqk 1/1 Running 0 2m2s kustomize-controller-644f79985c-tqbdp 1/1 Running 0 2m2s notification-controller-ddf44665d-r6q8f 1/1 Running 0 2m2s source-controller-9dfbc5cd-cbs9x 1/1 Running 0 2m2s tf-controller-7975f69b86-psdmx 1/1 Running 0 58s tf-controller-7975f69b86-s9pm5 1/1 Running 0 58s tf-controller-7975f69b86-w5bmt 1/1 Running 0 58s weave-gitops-enterprise-mccp-cluster-service-756cc58859-kqt45 1/1 Running 0 20s weave-gitops-enterprise-pipeline-controller-85bf5c67d6-459km 1/1 Running 0 76s

I could login

policy agent is reconciled but fails because cert-manager

after installing cert-manager, policy agent reconciles fine:

deployed terraform resource

i could see policy too

tested invalid path

--repo-path=clusters/management --components-extra=tf-controller2 ► creating client to cluster ✔ created client to cluster: https://127.0.0.1:51388 Error: cannot config bootstrap: cannot create components extra configuration: unsupported component selected: tf-controller2 exit status 1

enekofb · 2023-12-07T09:31:37Z

pkg/bootstrap/steps/components_extra.go

+}
+
+func installExtraComponents(input []StepInput, c *Config) ([]StepOutput, error) {
+	for _, param := range input {


review interactive experience:

expectations:

I should be able to skip installing

i should be able to select one

i should be able to select more than one

enekofb

Nice one!

Here the follow-ups to consider from the previous review #3647 (review)

waleedhammam added the enhancement New feature or request label Nov 19, 2023

waleedhammam force-pushed the 3276-cli-controllers branch 16 times, most recently from a049adf to 61e5aa5 Compare November 21, 2023 13:07

add controllers configurations with acceptance and unit tests

2409dc7

waleedhammam force-pushed the 3276-cli-controllers branch from 61e5aa5 to 2409dc7 Compare November 21, 2023 13:23

waleedhammam marked this pull request as ready for review November 21, 2023 13:27

waleedhammam requested review from enekofb and Samra10 November 21, 2023 13:27

enekofb reviewed Nov 28, 2023

View reviewed changes

enekofb self-requested a review November 28, 2023 09:53

waleedhammam added 4 commits November 28, 2023 16:21

silent mode doesn't ask for extra controllers

8da012a

use components-extra instead of extra-controllers

b8606fb

install components before check ui domain

2c5af01

remove example

33aae21

enekofb reviewed Dec 1, 2023

View reviewed changes

use empty quotes instead of none value

c2d46dc

waleedhammam added 9 commits December 6, 2023 13:44

fix flags

68ef627

add test case for bootstrap flux in silent mode

0fbf5e6

don't use yes as default and fail if no defaults set

c3b9291

Merge branch 'main' into 3276-cli-controllers

eec4102

remove capi controller and refactor configuration

7853602

add case for silent mode

0bfedba

add test case for silent mode

c56804f

rename comment

9e91b8c

remove capi leftovers

8306cc0

waleedhammam mentioned this pull request Dec 6, 2023

Onboarding CLI: investigate Capi controller #3698

Open

waleedhammam added 5 commits December 6, 2023 16:59

use policy agent from url in the repository

2c21cf3

Merge branch 'main' into 3276-cli-controllers

d3ba2d6

fix comment

30a4da3

add update logic to extra components

09c6e99

Merge branch 'main' into 3276-cli-controllers

f5ddec0

waleedhammam linked an issue Dec 6, 2023 that may be closed by this pull request

bootstrap cli: add mode to use defaults / non-interactive mode #3465

Closed

update acceptance testing wge version

a61370d

waleedhammam requested a review from enekofb December 6, 2023 16:35

waleedhammam added 3 commits December 7, 2023 08:57

Merge branch 'main' into 3276-cli-controllers

04a7f92

restore empty input instead of nil

2acd174

fix flux bootstrap

507dabe

waleedhammam force-pushed the 3276-cli-controllers branch from 839015c to 507dabe Compare December 7, 2023 07:34

enekofb reviewed Dec 7, 2023

View reviewed changes

enekofb added 2 commits December 7, 2023 10:59

some wording suggestions

eec87da

using none instead of empty string

b110a07

enekofb self-requested a review December 7, 2023 10:13

update messages

995d6bd

enekofb approved these changes Dec 7, 2023

View reviewed changes

waleedhammam merged commit 7a26044 into main Dec 7, 2023
10 checks passed

waleedhammam deleted the 3276-cli-controllers branch December 7, 2023 10:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add bootstrap CLI controllers configurations #3647

Add bootstrap CLI controllers configurations #3647

waleedhammam commented Nov 19, 2023 •

edited

Loading

enekofb left a comment

enekofb left a comment

enekofb left a comment

enekofb Dec 6, 2023

enekofb Dec 6, 2023

enekofb Dec 6, 2023

enekofb Dec 6, 2023

enekofb Dec 6, 2023

enekofb Dec 6, 2023

enekofb Dec 7, 2023

enekofb Dec 7, 2023

enekofb Dec 7, 2023

enekofb Dec 7, 2023

enekofb Dec 7, 2023

enekofb Dec 7, 2023

enekofb Dec 7, 2023

enekofb Dec 7, 2023

enekofb Dec 7, 2023

enekofb Dec 7, 2023

enekofb left a comment

	// if no default value select yes by default
	// if no default value an error will be returned

Add bootstrap CLI controllers configurations #3647

Add bootstrap CLI controllers configurations #3647

Conversation

waleedhammam commented Nov 19, 2023 • edited Loading

enekofb left a comment

Choose a reason for hiding this comment

enekofb left a comment

Choose a reason for hiding this comment

enekofb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enekofb left a comment

Choose a reason for hiding this comment

waleedhammam commented Nov 19, 2023 •

edited

Loading