Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Sketch out cluster generator in the CRD, and generator interface #2

Merged
merged 2 commits into from Jun 19, 2020

Conversation

dgoodwin
Copy link
Contributor

Builds on #1 to add a field for cluster generator with label selector.

Fixes the main.go compile problem, and adds dist/ to .gitignore.

Adds a simple interface for generators and two empty implementations for list and cluster.

@CLAassistant
Copy link

CLAassistant commented Jun 11, 2020

CLA assistant check
All committers have signed the CLA.

// The expected / desired list of Applications is returned, it then needs to be reconciled
// against the current state of the Applications in the cluster.
GenerateApplications(appSet *argoprojiov1alpha1.ApplicationSet) ([]argov1alpha1.Application, error)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it might be helpful to break this into an interface and implement for each type of generator, to help us work without causing git conflicts as much as possible.

Returns the Applications (desired state) to help with unit testing, something else will need to reconcile these with the actual state of the Applications currently.

Common code can be shared amongst generators using functions in this package, but my suspicion is the implementations of each generator will be unique enough to isolate them as separate implementations.

main.go Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Copy link
Member

@xianlubird xianlubird left a comment

Choose a reason for hiding this comment

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

LGTM.
You can wait for #1 to be merged, and your base code will also change.

@dgoodwin
Copy link
Contributor Author

Rebased onto latest changed and ready for review.

pkg/generators/list.go Outdated Show resolved Hide resolved
pkg/generators/list.go Outdated Show resolved Hide resolved
pkg/generators/cluster.go Outdated Show resolved Hide resolved
@xianlubird
Copy link
Member

What's your opinion about this pr @jessesuen

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants