Skip to content

Commit

Permalink
add more alternatives
Browse files Browse the repository at this point in the history
Signed-off-by: lsviben <sviben.lovro@gmail.com>
  • Loading branch information
lsviben committed Mar 8, 2023
1 parent ff175e9 commit 59c27f5
Showing 1 changed file with 23 additions and 0 deletions.
23 changes: 23 additions & 0 deletions design/one-pager-ignore-changes.md
Expand Up @@ -156,6 +156,12 @@ This will not prevent the field from appearing in the schema of the managed
resources. However, we will ignore the `spec.ignoreChanges` when the feature
is not enabled.
#### Alternative feature gating approach
To avoid changing the API, we could ship the `ignoreChanges` field as an
optional annotation as a first solution. This would allow us to survey
users and see how much this feature is useful before adding it to the API.
### Implementation alternative considered
An alternative was considered where the above helper function which unsets
Expand All @@ -174,6 +180,8 @@ change all the providers, it has a few drawbacks:
## Solution alternative considered
### initContainers
The idea here was to do something similar to K8s `initContainers` and add a
field `initProvider` to the managed resources. The parameters under
`initProvider` would be used only during creation.
Expand Down Expand Up @@ -214,11 +222,26 @@ initialization*, which sets some of the `forProvider`fields. To avoid
conflicts, the fields that are in `initProviders` would be ignored during
*late initialize*.
### Case by case
This approach would just tackle the ignored fields on a case-by-case basis,
instead of implementing a generic solution. The premise here is that not many
resources would need to use this feature.
For instance in the [NodeGroup] example, we would add a field `initialSize`,
along with the current `desiredSize` and the updates could be handled in code.
While this approach is valid, making changes case-by-case could lead to
bad user experience as they would need time to identify the issue and wait
for provider code change, instead of just setting a field. On the other hand,
if this issue is not that widespread, we could have an easy fix.
[AutoScalingGroup DesireCapacity]: https://doc.crds.dev/github.com/upbound/provider-aws/autoscaling.aws.upbound.io/AutoscalingGroup/v1beta1@v0.21.0#spec-forProvider-desiredCapacity
[DynamoDB Tables]: https://doc.crds.dev/github.com/upbound/provider-aws/dynamodb.aws.upbound.io/Table/v1beta1@v0.24.0
[Policy]: https://doc.crds.dev/github.com/upbound/provider-aws/appautoscaling.aws.upbound.io/Policy/v1beta1@v0.24.0
[Target]: https://doc.crds.dev/github.com/upbound/provider-aws/appautoscaling.aws.upbound.io/Target/v1beta1@v0.24.0
[NodeGroup]: https://doc.crds.dev/github.com/upbound/provider-aws/eks.aws.upbound.io/NodeGroup/v1beta1@v0.24.0
[Node Pool]: https://doc.crds.dev/github.com/upbound/provider-gcp/container.gcp.upbound.io/NodePool/v1beta1@v0.28.0
[ignore_changes]: https://developer.hashicorp.com/terraform/language/meta-arguments/lifecycle#ignore_changes
[crossplane-runtime]: https://github.com/crossplane/crossplane-runtime/
[crossplane-runtime/fieldpath]: https://github.com/crossplane/crossplane-runtime/tree/master/pkg/fieldpath

0 comments on commit 59c27f5

Please sign in to comment.