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.

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

Creates alternative kustomize installer for installation of the camel-k operator #2284

Merged
merged 10 commits into from
Sep 22, 2021

Conversation

phantomjinx
Copy link
Contributor

@phantomjinx phantomjinx commented May 11, 2021

Re-layout config directory to implement a declarative kustomize installer.

Release Note

An alternative kustomize based installer that applies the kubernetes resources using `kustomize` & `kubectl`.

The `config` directory contains all the resources for installation and configuration of the camel-k operator. While, by default, those resource are installed through the `kamel` binary, they can also be applied directly to a cluster using `kustomize`. These resources can be modified prior to their installation and the `kustomize.yaml` files be changed to include extra patches and settings, as required.

A Makefile is included to provide an imperative guide to the otherwise declarative structure. The sequence is thus:

1. `make setup` (must be executed as cluster-admin)
2. `make operator`
3. `make platform` (if a customized integration-platform is required)
4. `make example` (optional - installs the `hello-world` camel-k integration)

Each of these Makefile rules have their own extra environment variables that can be customized. Please review the rules in the Makefile for details.

@phantomjinx
Copy link
Contributor Author

@astefanutti
Rewrite in accordance with discussions.
Fixes to ensure e2e tests work and pass (hopefully!)

@astefanutti
Copy link
Member

@phantomjinx wow, that looks fantastic! I love how you addressed patching, Role (resp. RoleBinding) auto-promoting to ClusterRole (resp. ClusterRoleBinding), and that you fixed #2167 in the process 👍🏼.

I'd like to test it locally to give you more feedback on the Kustomize directory structure, even if that looks great at first glance. But if you prefer, we can have this merged once CI is green, and iterate later on. Whatever option we choose, may I suggest to create a new GitHub Actions workflow called kustomize, that would install Camel K with plain kustomize, and run some tests. Ideally, If we were confident enough, we could even migrate an existing workflow to using plain kustomize, but that would require to refactor the way Camel K is installed from the tests themselves.

@astefanutti astefanutti added area/installation Installation and Topology kind/feature New feature or request labels May 18, 2021
@phantomjinx
Copy link
Contributor Author

Thanks....

e2e tests still failing (oddly!) so will run back through those again.

@astefanutti
Copy link
Member

Yes GH Actions didn't seem to be very healthy. I've restarted the failing workflows 🤞🏼.

@astefanutti
Copy link
Member

GH Actions are experiencing an incident: https://www.githubstatus.com/incidents/m16jzl31gnqt.

@phantomjinx
Copy link
Contributor Author

Hoepfully, the openshift failing test is just a one-off since it passed fine here -> https://github.com/phantomjinx/camel-k/actions/runs/853465524

@astefanutti
Copy link
Member

Right, the failure seems unrelated. I've restarted it 🤞🏼.

@phantomjinx
Copy link
Contributor Author

Already seeing conflicts on this so will need to rebase. However, am now adding e2e tests so I'll finish those before rebasing.

@phantomjinx phantomjinx force-pushed the kustomize branch 2 times, most recently from f3016ea to 2a00c28 Compare June 4, 2021 16:05
@phantomjinx
Copy link
Contributor Author

Rebased and now includes e2e tests for kustomize. Currently included in kubernetes.yml action but could be spun out if so desired.

@phantomjinx
Copy link
Contributor Author

All checks passed! :-)

Copy link
Member

@astefanutti astefanutti left a comment

Choose a reason for hiding this comment

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

I'd like to keep the config directory structure as simple as possible, and as close to the Kustomize way as possible. It is important for developers that maintain the configuration, and for users relying on it. I find it a bit difficult with the current structure to understand the graph of resources (thinking how to use this without the Makefile can help).

Here are a few propositions that I have in mind:

  • Merge the manager and the operator directories (maybe keep manager which is the Kustomize way)
  • "Revert" the infrastructure directory: move the SA back into the manager directory, move the RBAC resources into the main rbac directory
  • Flatten the structure as much as possible: simplify the rbac directory, move patches into the same directory, rather than creating a dedicated directory.

I've been reviewing this "interruptibly", so I apologise if the review sounds inconsistent or even off 😅.

config/Makefile Outdated Show resolved Hide resolved
.github/workflows/builder.yml Outdated Show resolved Hide resolved
e2e/builder/build_test.go Outdated Show resolved Hide resolved
config/rbac/kubernetes/operator-role-binding-events.yaml Outdated Show resolved Hide resolved
e2e/common/kustomize/operator_test.go Show resolved Hide resolved
e2e/common/kustomize/operator_test.go Outdated Show resolved Hide resolved
config/Makefile Outdated Show resolved Hide resolved
config/Makefile Outdated Show resolved Hide resolved
@phantomjinx
Copy link
Contributor Author

I'd like to keep the config directory structure as simple as possible, and as close to the Kustomize way as possible. It is important for developers that maintain the configuration, and for users relying on it. I find it a bit difficult with the current structure to understand the graph of resources (thinking how to use this without the Makefile can help).

Noted. Maybe an ancillary directory structure targetting install-type operations would be more transparent?
I am concerned about the differentiation between openshift and kubernetes resources. The 'mutually exclusive' question I have eluded to elsewhere but will determine what we do about this aspect of the directory structure. The original kustomize way makes no provision for this.

Here are a few propositions that I have in mind:

* Merge the `manager` and the `operator` directories (maybe keep `manager` which is the Kustomize way)

The operator directory's kustomize.yaml does more than the manager directory since its adds the infrastructure resources as well. They cannot be added to manager since they are operand-related and not operator-related.

* "Revert" the `infrastructure` directory: move the SA back into the `manager` directory, move the RBAC resources into the main `rbac` directory

Discussed elsewhere but to summarize, these are operand resources that the operator should be installing rather than resources required to get the operator installed. These 2 concepts are fundamentally different and that division should be maintained, in my view.

* Flatten the structure as much as possible: simplify the `rbac` directory, move patches into the same directory, rather than creating a dedicated directory.

I can certainly run with that philosophy for the main config directory. Anything that requires 'Makefile-magic' I can migrate to the 'install' directory instead.

I've been reviewing this "interruptibly", so I apologise if the review sounds inconsistent or even off sweat_smile.

Absolutely, not a problem. This is a longstanding issue with some rather knotty problems so happy to label this WIP and we'll keep returning to it, iteratively.

…flag

* When Global flag is used, the cluster-role-openshift conflicts with the
  promoted role so renames it to cluster-role-console-openshift

* Updates all source relating to new cluster-role name

* Re-generated resources
* Builder service account and related roles are operand managed by the
  operator and so are resources rather than configuration/install time
  artifacts
@phantomjinx phantomjinx force-pushed the kustomize branch 2 times, most recently from a9b7691 to 36fe3be Compare September 15, 2021 13:40
Copy link
Member

@astefanutti astefanutti left a comment

Choose a reason for hiding this comment

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

Would it be possible to merge the install directory into the config directory, at least for the manifests / patches, to follow Kustomize conventions, and to have it working with plain Kustomize / kubectl apply -k.

I wonder, would you think ultimately something like curl https://github.com/apache/camel-k/releases/download/$VERSION/installer.tar.gz | tar xvz | kubectl apply -k - would be possible? Maybe we could produce default YAMLs as part of the release, with default configurations for Kubernetes and OpenShift, to have like curl https://github.com/apache/camel-k/releases/download/$VERSION/openshift-install.yaml | kubectl apply -k -.

Once we have the structure right, I think it'll be good to go, and we'll be able to iterate subsequently on the testing and think about how to replace kamel install.

cmd/util/doc-gen/main.go Outdated Show resolved Hide resolved
config/rbac/kustomization.yaml Outdated Show resolved Hide resolved
install/Makefile Outdated Show resolved Hide resolved
@phantomjinx
Copy link
Contributor Author

phantomjinx commented Sep 17, 2021

Would it be possible to merge the install directory into the config directory, at least for the manifests / patches, to follow Kustomize conventions, and to have it working with plain Kustomize / kubectl apply -k.

I have migrated all the patches over to the config directory but I think we should go with your original suggestion of keeping the install directory separate so as to avoid confusion with the kustomize layout. The install directory can be used to "drive" the automation of the kustomize install.

I wonder, would you think ultimately something like curl https://github.com/apache/camel-k/releases/download/$VERSION/installer.tar.gz | tar xvz | kubectl apply -k - would be possible? Maybe we could produce default YAMLs as part of the release, with default configurations for Kubernetes and OpenShift, to have like curl https://github.com/apache/camel-k/releases/download/$VERSION/openshift-install.yaml | kubectl apply -k -.

I do not think this is possible at the moment. The biggest obstacle is that the namespace of the service-account listed in the bindings is by default 'placeholder'. Therefore, I use the Makefile to call sed to change this. If this were fixed in kustomize so that 'kustomize set namespace' updated it would be worth doing.

Once we have the structure right, I think it'll be good to go, and we'll be able to iterate subsequently on the testing and think about how to replace kamel install.

Copy link
Member

@astefanutti astefanutti left a comment

Choose a reason for hiding this comment

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

I've just left a couple of small comments. We can have this merged then as an initial version, and iterate further based on usage feedback.

config/samples/patch-integration-platform.yaml Outdated Show resolved Hide resolved
.github/workflows/knative.yml Outdated Show resolved Hide resolved
e2e/support/test_support.go Outdated Show resolved Hide resolved
e2e/support/test_support.go Outdated Show resolved Hide resolved
e2e/support/test_support.go Outdated Show resolved Hide resolved
resources/builder/builder-role-binding-core.yaml Outdated Show resolved Hide resolved
@phantomjinx phantomjinx force-pushed the kustomize branch 2 times, most recently from e36770b to ab70702 Compare September 20, 2021 12:35
Copy link
Member

@astefanutti astefanutti left a comment

Choose a reason for hiding this comment

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

Let's merge it once CI passes. Thanks a lot for the awesome work!

* script/platform-check.sh
* cmd/util/platform-check
 * Determines the type of cluster user currently is accessing, namely
   openshift or kubernetes

* vfs-gen
 * Adds extra filters for ignoring files that should not be in resources

* config/...
 * Tidies up directory and resources
 * Moves openshift-only resources into a kustomize-handled directory of
   their own - easier to manage
 * Adds scorecard tests for use in bundle
 * Prometheus monitoring to bundle
  * This breaks installation of the operator via OLM due to attempts to
    apply PromethusRule fail with no CRD found for monitoring.coreos.com/v1
  * True for e2e tests using kind but not on Openshift
  * Prometheus rules should be installed separately where users are sure the
    environment supports them

* install/...
 * Adds kustomize installer provisioning
 * make example: installing example camel-k integration
 * make operator: installing camel-k operator
 * make platform: installing camel-k integration platform
 * make setup-cluster: installing cluster-wide CRD & RBAC resources only
 * make setup: installing RBAC resources & calls setup-cluster
 * Makefile provides sciptable auto-configuration of kustomize files
 * kustomize can be executed over directories manually

* pkg/...
 * Fixes typo in schema_types.go - causes warning in bundle generation
 * Updates resources names and paths

* script/Makefile
 * Originally, considered allowing overriding of IMAGE_NAME & VERSION but
   this 'loses' the default values which are required for the
  `kustomize set image ...` command.
 * Revert overriding of IMAGE_NAME & VERSION
 * Allows easy updating of ancillary tools versions & downloads them if
   not available
 * Adds CUSTOM_IMAGE & CUSTOM_VERSION that default to IMAGE_NAME & VERSION
   but can be overridden, whilst also preserving original values

* PROJECT
 * Updates PROJECT to version 3 due to use of operator-sdk 1.5.0
* upgrade.yml
 * Remove operator-sdk step as taken cared of in Makefile
 * Updates build variables to use CUSTOM_IMAGE
 * Replaces kustomize patching of 'replaces' property with sed as the
   manifest is no longer included in the kustomize.yaml

* e2e/...
 * Tests for kustomize install
* config/manager
 * permanent default patches for node-toleration, node-selector and resource
   requirements always applied in their default state
 * pull-policy, ports and watch-namespace patches avalailable to be applied
   if user wishes to manually add them

* config/rbac
 * role promotion patches available to be applied if user wishes to manually
   add them

* config/samples
 * integration platform patch always applied but empty so user can add
   additional properties

* install
 * Moves all patches out of directory so while this may drive and automated
   install, it is possible to do a manual kustomize install using just the
   config directory

# Conflicts:
#	pkg/resources/resources.go
* Role installed by operator so not applicable to the kustomize install
@astefanutti
Copy link
Member

Well done! Thanks.

@heiko-braun
Copy link

Congrats! It has been a long way :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/installation Installation and Topology kind/feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants