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

CronJob and ApiServer source usage needs to updated Eventing 0.13.0 #609

Closed
rhuss opened this issue Jan 14, 2020 · 9 comments
Closed

CronJob and ApiServer source usage needs to updated Eventing 0.13.0 #609

rhuss opened this issue Jan 14, 2020 · 9 comments
Assignees
Labels
kind/proposal Issues or PRs related to proposals. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@rhuss
Copy link
Contributor

rhuss commented Jan 14, 2020

As a CronJobSource has been removed from Eventing v1alpha1 in favour of managing a CronJob resource directly together with a SinkBinding, we need to act before moving the dependency from Eventing 0.11.0 to 0.12.0

Possible actions:

  1. Remove kn source cronjob completely from kn
  2. Move to manage a CronJob and SinkBinding to cover the current feature which was about managing a CronJobSource only. The problem here is that managing a CronJob is not part of the Knative API contract, so the question is how this could be performed under within kn. @n3wscott any suggestions on how managing a CronJob source could be done purely with Knative API calls?

I'm afraid we have to jump on option 1 and delegate the management of a CronJob source to a plugin. Fully managing (crud operation) for source referring to podspecable (i.e. also managing those podsceables) becomes then out of scope for kn.

We need to adjust the Eventing MVP spec accordingly (i.e. removing CronJobSource and ContainerSource from the list of managed builtin source, leaving only the ApiServerSource).

@rhuss rhuss added kind/feature New feature or request kind/proposal Issues or PRs related to proposals. and removed kind/feature New feature or request labels Jan 14, 2020
@rhuss
Copy link
Contributor Author

rhuss commented Jan 14, 2020

Ok, I have seen that CronJoboSource has been moved to legacysources.v1alpha1. How long will they be available in core eventing (and the operators behind them) ?

@rhuss
Copy link
Contributor Author

rhuss commented Jan 14, 2020

So, sources.eventing.knative.dev remains the group for the legacy reference (we would just to need update the imports) and sources.knative.dev is the new group without CronJobSource. So for the next version we could just updated, but the principle question remains: What to do with cronjob support in kn ?

@rhuss
Copy link
Contributor Author

rhuss commented Jan 23, 2020

Since yesterday the direction seems to be clear:

  • We ship CronJobSource with kn 0.12 (thanks @n3wscott for porting over to the legacy client)
  • In 0.13 we will support in addition PingSource (in a different API group), which is essentially a renaming of CronJobSource.

The question yet is though if the old CronJobSource needs to be supported in 0.13 as according to the Knative Release Principles we need to support Knative Eventing 0.10 ... 0.13 and for 0.10 - 0.12 we only have CronJobSource.

Technically the tricky parts to have both, CronJobSource and PingSource:

  • Depending against which version of a Knative eventing kn is operating against, either source couldn't be available. In this case an error should be printed to point to the alternative.
  • If we move the code dependency of eventing to 0.13 and CronJobSource has been removed there, we have to manually pull in parts of 0.12 (probably by copying it over literally into the kn repository). @n3wscott any thoughts on this how this could be achieved more elegantly ?

@rhuss
Copy link
Contributor Author

rhuss commented Jan 24, 2020

Another option would be to keep kn source cronjob but use internally a PingSource for 0.13. This would allow us to keep the UX stable but still move on and don't have to deal with split dependencies on eventing. We could even keep --schedule mandatory for kn source cronjob while for kn source ping its optional (with a default to "* * * * *").

So my prefered options for 0.13 would be:

  • Remove kn source cronjob

or

  • Keep kn source cronjob in addition to kn source ping but let it use PingSource.

@navidshaikh
Copy link
Collaborator

Remove kn source cronjob

+1

Let's represent PingSource as way to go for scheduled events use cases.

Keep kn source cronjob in addition to kn source ping but let it use PingSource.

This could add confusion in client UX for users.
If we're to keep CronJob source, we should create equivalent kind (though kn would then need to identify if legacyclient to be used or otherwise).

@rhuss rhuss self-assigned this Feb 5, 2020
@rhuss rhuss changed the title kn source cronjob needs complete rework CronJob and ApiServer source usage needs to updated Eventing 0.13.0 Feb 7, 2020
@navidshaikh
Copy link
Collaborator

/close

@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 15, 2020
@navidshaikh
Copy link
Collaborator

/close

@knative-prow-robot
Copy link
Contributor

@navidshaikh: Closing this issue.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/proposal Issues or PRs related to proposals. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

3 participants