-
Notifications
You must be signed in to change notification settings - Fork 153
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
36/wego add command #106
36/wego add command #106
Conversation
cmd/wego/add/cmd.go
Outdated
` | ||
|
||
// Will move into filesystem when we store wego infrastructure in git | ||
const appCRD = `apiVersion: apiextensions.k8s.io/v1beta1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you are creating a crd here, should we then create the story to start implementing the wego controller?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can, but I'm still not sure what to put in it yet. I had the choice to either tell flux to ignore app.yaml
or turn it into a CRD so that flux wouldn't fail on it. I figured I'd go ahead and make the CRD now...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way I see could be like: wego add
only drops the app.yaml
there and nothing more, it would be the controller's responsibility to create the source and kustomization|helm custom resources, as a default behavior. That way we kinda abstract away flux entirely. In case the user decides they want more control, we would allow them to override the flux crs, by dropping them into the repo.
for now, we can keep the command adding those flux crs so we have a working command, but as soon we have the controller in place we could let the controller manage them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense to me. In particular, we can consult the app.yaml to determine which controllers a particular app needs
…fallout from rebase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Provides a first cut of
wego add
.