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

Implement Provider controller #408

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
Draft

Conversation

alexgeorgousis
Copy link
Contributor

@alexgeorgousis alexgeorgousis commented Dec 5, 2024

Closes #332

Tasks

  • Deploy complete provider Deployment from the controller
    • Deploy ConfigMap (introduced in Step 4) needed by Deployment, Should the controller deploy this or should we let users optionally deploy it separately as a way to configure a provider? This will be managed using Env vars within the deployment, to avoid the need to manage another configmap resource.
    • Use state transition logic like in other controllers (though we don't this in the runconfiguration controller, we just have a TODO to do it) Would require a rework of the state transition logic to not be tied to workflows, this should be done separately as it will be a large piece of work
  • Remove provider helm chart (it should now be redundant)
  • Test controller creates Deployment successfully
  • Create constant for owner label key
  • Handle updates to Provider resources
  • Set SynchronizationState on Provider resource
    • Write decoupled tests for all state transitions
    • Current problematic scenario: failing deployment and then fixing it (add a test case for it)
  • Write acceptance tests (see what we do for other reconcilers)
    • Ideally we wanna test all state transition cases, e.g. 1. create provider, 2. creates deployment, 3. deployment fails, 4. provider synchstate = failed
  • Set Synchronized condition (we do this for all other resources)???
  • What about ObservedGeneration? (It's on the CRD but we never set it explicitly)
  • Set events on failure? Check if we do this for our other custom resources
  • Make sure provider type isn't used anywhere and, if not, remove it from the CRD (and wherever else it's currently referenced)
  • Move Provider permissions from manager-providers-viewer-rolebinding.yaml to roles.yaml
  • Ensure all logging in the provider controller code is at the correct level
  • Since this PR has changed the Provider CRD, do we need a v1alpha7 release?
    • Potentially not, because we haven't released v1alpha6 yet, so we can add it to it
  • Update documentation

Useful Links

@alexgeorgousis alexgeorgousis force-pushed the provider-controller branch 6 times, most recently from ee146de to 7ca4231 Compare December 9, 2024 11:30
type ProviderStatus struct {
Conditions Conditions `json:"conditions,omitempty"`
Spec ProviderSpec `json:"spec,omitempty"`
Status Status `json:"status,omitempty"`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We previously used a dedicated ProviderStatus struct for the provider CRD status, but we didn't see the point in it because:

  • It only has a single field, Conditions, which is part of the default Status struct already
  • We don't set the provider status anywhere (except for tests, just so they compile)

So we removed ProviderStatus and just used the default Status struct.

@alexgeorgousis alexgeorgousis force-pushed the provider-controller branch 9 times, most recently from 94500b9 to c555c38 Compare December 12, 2024 10:27
@grahamia grahamia force-pushed the provider-controller branch from 1b5556a to e477814 Compare December 16, 2024 10:49
@alexgeorgousis alexgeorgousis force-pushed the provider-controller branch 9 times, most recently from 90605f7 to c98ef2d Compare December 27, 2024 10:26
@alexgeorgousis alexgeorgousis force-pushed the provider-controller branch 2 times, most recently from 4e11412 to 014c743 Compare December 30, 2024 14:57
@grahamia grahamia force-pushed the provider-controller branch from aa9ce00 to ddef6a9 Compare January 10, 2025 11:08
@alexgeorgousis alexgeorgousis force-pushed the provider-controller branch 6 times, most recently from b293936 to 6d1002c Compare January 16, 2025 15:53
@alexgeorgousis alexgeorgousis force-pushed the provider-controller branch 7 times, most recently from b257e07 to 9feb559 Compare January 17, 2025 09:51
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.

Provider Controller (Step 5)
5 participants