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

Break provider-aws up by service #680

Merged
merged 7 commits into from May 2, 2023

Conversation

ulucinar
Copy link
Collaborator

@ulucinar ulucinar commented Apr 26, 2023

Description of your changes

Relevant proposal: crossplane/crossplane#3939
Depends on: crossplane/upjet#194

This PR proposes the following changes according to the proposal crossplane/crossplane#3939:

  • Subpackages belonging to each API group is produced. An example is: provider-aws-kms.
  • ProviderConfig, ProviderConfigUsage and StoreConfig are part of a config package named provider-aws-config.
  • The monolith package (containing all the CRDs and associated controllers) is still produced.
  • Each produced package except for the monolith package has the pkg.crossplane.io/provider-family label in its package metadata.
  • Each service package except for the config package declares a dependency to the config package.

It also introduces the SUBPACKAGES make variable so that it's possible to build the individual subpackages. An example for building is:

make -j 3 SUBPACKAGES="monolith config ec2" build.all

One can also use the same variable to push those (already built) packages:

make -j 3 SUBPACKAGES="monolith config ec2" BRANCH_NAME=main publish

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Successfully built and pushed the monolith, config and ec2 subpackages.

Also successfully provisioned and destroyed an ec2.VPC instance using the package .../upbound/provider-aws-ec2:v0.34.0-rc.0.20.ga26202f1.

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we know of a lightweight templating tool that we can quickly integrate into our build pipelines?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can use gomplate. It is already available in the build submodule (we may want to move it out of local.mk though).

Copy link
Collaborator Author

@ulucinar ulucinar Apr 27, 2023

Choose a reason for hiding this comment

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

Thanks @turkenh for the suggestion, makes sense to me. Let's proceed as separate manifests for now and once we have the installation of gomplate refactored into somewhere like the k8s_tools.mk, then we can utilize the tool for templating the crossplane.yaml.

spec:
dependsOn:
- provider: {{ XPKG_REG_ORGS }}/provider-aws-config
version: ">={{ VERSION }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

As we discussed offline, we may want to soften the requirement here. Consider the following scenario:

  1. Installed provider-aws-ec2:v0.50.0.
  2. Config package provider-aws-config:v0.50.0 installed via package manager.
  3. Upgraded ec2 package to provider-aws-ec2:v0.51.0
  4. Package manager will start failing since the dependency to the config package (>=v0.51.0 is not satisfied unless the config package is upgraded to that version manually.

I believe we don't want to introduce such a constraint and we should instead use >=v0.0.0 here. One caveat, in that case, would be working with rc versions though as they would still bring the latest stable as dependency 🤔

@ulucinar
Copy link
Collaborator Author

/test-examples="examples/ec2/vpc.yaml"

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
- Default the SUBPACKAGES make var to only "monolith"

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
package/crossplane.yaml Outdated Show resolved Hide resolved
Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
Copy link
Contributor

@ezgidemirel ezgidemirel left a comment

Choose a reason for hiding this comment

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

I have updated the platform-ref-aws configuration with the sub-packages and successfully installed it. There are 160 CRDs installed to the cluster and I could successfully create the cluster claim.

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
@ulucinar ulucinar merged commit 6caff78 into crossplane-contrib:main May 2, 2023
8 of 9 checks passed
@ulucinar ulucinar deleted the aws-family branch May 2, 2023 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants