You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by Lajith Koova <la...@gmail.com> on 2024/05/01 07:05:32 UTC
Discussion: Condition field in the CR status
Hello,
Starting discussion thread here to discuss a proposal to add Conditions
field in the CR status of Flink Deployment and FlinkSessionJob.
Here is the google doc with details. Please provide your thoughts/inputs.
https://docs.google.com/document/d/12wlJCL_Vq2KZnABzK7OR7gAd1jZMmo0MxgXQXqtWODs/edit?usp=sharing
Thanks
Lajith
Re: Discussion: Condition field in the CR status
Posted by Lajith Koova <la...@gmail.com>.
Thanks David for the feedback. Yes you are right. We can provide
meaningful information in reason rather than provide it as "Ready".
Message will have more detailed information .
Also updated document on how to handle multiple conditions in status.
Thanks
Lajith
On Mon, May 13, 2024 at 4:00 PM David Radley <da...@uk.ibm.com>
wrote:
> Hi Lajith,
> This idea for a Flip is a good addition, which I support.
>
> As discussed:
>
> These conditions populate a status field in the K8s UI – Openshift in our
> case. Currently the status does not contain any information. With the
> conditions present, the status will be populated with meaningful
> information on the UI; which means the readiness is explicitly shown on the
> UI improving the users experience .
>
> One other observation, in your example :
>
> reason: Ready
>
> status: 'True'
>
> type: Ready
>
>
>
> Could reason be something more granular to give more information about
> Ready status, maybe rolledback, deployed or stable?
>
> We also talked of whether there are any use cases would be beneficial, at
> this time we don’t think so – but you said you have a look at this,
>
> Kind regards, David,
>
>
> From: Lajith Koova <la...@gmail.com>
> Date: Monday, 13 May 2024 at 10:29
> To: dev@flink.apache.org <de...@flink.apache.org>
> Subject: [EXTERNAL] Re: Discussion: Condition field in the CR status
> Thanks for the feedback Jeyhun.
>
> Regarding Will the usage of Conditions be enabled by default? Or will there
> be any
> disadvantages for Flink users . Yes the conditions will be enabled by
> default for the CR.
>
> You are right , when there are multiple conditions of the same type,
> approach is we will override the old condition with new conditions only if
> conditions status and messages are the same. If they are different , we
> will add to existing conditions.
>
> Thanks
> Lajith
>
> On Fri, May 3, 2024 at 8:05 PM Jeyhun Karimov <je...@gmail.com>
> wrote:
>
> > Hi Lajith,
> >
> > Thanks a lot for driving this FLIP. Please find my comments below:
> >
> > - I echo Gyula that including some examples and further explanations
> might
> > ease reader's work. With the current version, the FLIP is a bit hard to
> > follow.
> >
> > - Will the usage of Conditions be enabled by default? Or will there be
> any
> > disadvantages for Flink users?
> >
> > If Conditions with the same type already exist in the Status Conditions
> > > list, then replace the existing condition with the same type if the
> > > Condition status and message are different.
> >
> > - Do you think we should have clear rules about handling rules for how
> > these Conditions should be managed, especially when multiple Conditions
> of
> > the same type are present?
> > For example, resource has multiple causes for the same condition (e.g.,
> > Error due to network and Error due to I/O). Then, overriding the old
> > condition with the new one is not the best approach no?
> > Please correct me if I misunderstood.
> >
> > Regards,
> > Jeyhun
> >
> > On Fri, May 3, 2024 at 8:53 AM Gyula Fóra <gy...@gmail.com> wrote:
> >
> > > Hi Lajith!
> > >
> > > Can you please include some examples in the document to help reviewers?
> > > Just some examples with the status and the proposed conditions.
> > >
> > > Cheers,
> > > Gyula
> > >
> > > On Wed, May 1, 2024 at 9:06 AM Lajith Koova <la...@gmail.com>
> wrote:
> > >
> > > > Hello,
> > > >
> > > >
> > > > Starting discussion thread here to discuss a proposal to add
> Conditions
> > > > field in the CR status of Flink Deployment and FlinkSessionJob.
> > > >
> > > >
> > > > Here is the google doc with details. Please provide your
> > thoughts/inputs.
> > > >
> > > >
> > > >
> > > >
> > >
> >
> https://docs.google.com/document/d/12wlJCL_Vq2KZnABzK7OR7gAd1jZMmo0MxgXQXqtWODs/edit?usp=sharing
> > > >
> > > >
> > > > Thanks
> > > > Lajith
> > > >
> > >
> >
>
> Unless otherwise stated above:
>
> IBM United Kingdom Limited
> Registered in England and Wales with number 741598
> Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
>
RE: Discussion: Condition field in the CR status
Posted by David Radley <da...@uk.ibm.com>.
Hi Lajith,
This idea for a Flip is a good addition, which I support.
As discussed:
These conditions populate a status field in the K8s UI – Openshift in our case. Currently the status does not contain any information. With the conditions present, the status will be populated with meaningful information on the UI; which means the readiness is explicitly shown on the UI improving the users experience .
One other observation, in your example :
reason: Ready
status: 'True'
type: Ready
Could reason be something more granular to give more information about Ready status, maybe rolledback, deployed or stable?
We also talked of whether there are any use cases would be beneficial, at this time we don’t think so – but you said you have a look at this,
Kind regards, David,
From: Lajith Koova <la...@gmail.com>
Date: Monday, 13 May 2024 at 10:29
To: dev@flink.apache.org <de...@flink.apache.org>
Subject: [EXTERNAL] Re: Discussion: Condition field in the CR status
Thanks for the feedback Jeyhun.
Regarding Will the usage of Conditions be enabled by default? Or will there
be any
disadvantages for Flink users . Yes the conditions will be enabled by
default for the CR.
You are right , when there are multiple conditions of the same type,
approach is we will override the old condition with new conditions only if
conditions status and messages are the same. If they are different , we
will add to existing conditions.
Thanks
Lajith
On Fri, May 3, 2024 at 8:05 PM Jeyhun Karimov <je...@gmail.com> wrote:
> Hi Lajith,
>
> Thanks a lot for driving this FLIP. Please find my comments below:
>
> - I echo Gyula that including some examples and further explanations might
> ease reader's work. With the current version, the FLIP is a bit hard to
> follow.
>
> - Will the usage of Conditions be enabled by default? Or will there be any
> disadvantages for Flink users?
>
> If Conditions with the same type already exist in the Status Conditions
> > list, then replace the existing condition with the same type if the
> > Condition status and message are different.
>
> - Do you think we should have clear rules about handling rules for how
> these Conditions should be managed, especially when multiple Conditions of
> the same type are present?
> For example, resource has multiple causes for the same condition (e.g.,
> Error due to network and Error due to I/O). Then, overriding the old
> condition with the new one is not the best approach no?
> Please correct me if I misunderstood.
>
> Regards,
> Jeyhun
>
> On Fri, May 3, 2024 at 8:53 AM Gyula Fóra <gy...@gmail.com> wrote:
>
> > Hi Lajith!
> >
> > Can you please include some examples in the document to help reviewers?
> > Just some examples with the status and the proposed conditions.
> >
> > Cheers,
> > Gyula
> >
> > On Wed, May 1, 2024 at 9:06 AM Lajith Koova <la...@gmail.com> wrote:
> >
> > > Hello,
> > >
> > >
> > > Starting discussion thread here to discuss a proposal to add Conditions
> > > field in the CR status of Flink Deployment and FlinkSessionJob.
> > >
> > >
> > > Here is the google doc with details. Please provide your
> thoughts/inputs.
> > >
> > >
> > >
> > >
> >
> https://docs.google.com/document/d/12wlJCL_Vq2KZnABzK7OR7gAd1jZMmo0MxgXQXqtWODs/edit?usp=sharing
> > >
> > >
> > > Thanks
> > > Lajith
> > >
> >
>
Unless otherwise stated above:
IBM United Kingdom Limited
Registered in England and Wales with number 741598
Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU
Re: Discussion: Condition field in the CR status
Posted by Lajith Koova <la...@gmail.com>.
Thanks for the feedback Jeyhun.
Regarding Will the usage of Conditions be enabled by default? Or will there
be any
disadvantages for Flink users . Yes the conditions will be enabled by
default for the CR.
You are right , when there are multiple conditions of the same type,
approach is we will override the old condition with new conditions only if
conditions status and messages are the same. If they are different , we
will add to existing conditions.
Thanks
Lajith
On Fri, May 3, 2024 at 8:05 PM Jeyhun Karimov <je...@gmail.com> wrote:
> Hi Lajith,
>
> Thanks a lot for driving this FLIP. Please find my comments below:
>
> - I echo Gyula that including some examples and further explanations might
> ease reader's work. With the current version, the FLIP is a bit hard to
> follow.
>
> - Will the usage of Conditions be enabled by default? Or will there be any
> disadvantages for Flink users?
>
> If Conditions with the same type already exist in the Status Conditions
> > list, then replace the existing condition with the same type if the
> > Condition status and message are different.
>
> - Do you think we should have clear rules about handling rules for how
> these Conditions should be managed, especially when multiple Conditions of
> the same type are present?
> For example, resource has multiple causes for the same condition (e.g.,
> Error due to network and Error due to I/O). Then, overriding the old
> condition with the new one is not the best approach no?
> Please correct me if I misunderstood.
>
> Regards,
> Jeyhun
>
> On Fri, May 3, 2024 at 8:53 AM Gyula Fóra <gy...@gmail.com> wrote:
>
> > Hi Lajith!
> >
> > Can you please include some examples in the document to help reviewers?
> > Just some examples with the status and the proposed conditions.
> >
> > Cheers,
> > Gyula
> >
> > On Wed, May 1, 2024 at 9:06 AM Lajith Koova <la...@gmail.com> wrote:
> >
> > > Hello,
> > >
> > >
> > > Starting discussion thread here to discuss a proposal to add Conditions
> > > field in the CR status of Flink Deployment and FlinkSessionJob.
> > >
> > >
> > > Here is the google doc with details. Please provide your
> thoughts/inputs.
> > >
> > >
> > >
> > >
> >
> https://docs.google.com/document/d/12wlJCL_Vq2KZnABzK7OR7gAd1jZMmo0MxgXQXqtWODs/edit?usp=sharing
> > >
> > >
> > > Thanks
> > > Lajith
> > >
> >
>
Re: Discussion: Condition field in the CR status
Posted by Jeyhun Karimov <je...@gmail.com>.
Hi Lajith,
Thanks a lot for driving this FLIP. Please find my comments below:
- I echo Gyula that including some examples and further explanations might
ease reader's work. With the current version, the FLIP is a bit hard to
follow.
- Will the usage of Conditions be enabled by default? Or will there be any
disadvantages for Flink users?
If Conditions with the same type already exist in the Status Conditions
> list, then replace the existing condition with the same type if the
> Condition status and message are different.
- Do you think we should have clear rules about handling rules for how
these Conditions should be managed, especially when multiple Conditions of
the same type are present?
For example, resource has multiple causes for the same condition (e.g.,
Error due to network and Error due to I/O). Then, overriding the old
condition with the new one is not the best approach no?
Please correct me if I misunderstood.
Regards,
Jeyhun
On Fri, May 3, 2024 at 8:53 AM Gyula Fóra <gy...@gmail.com> wrote:
> Hi Lajith!
>
> Can you please include some examples in the document to help reviewers?
> Just some examples with the status and the proposed conditions.
>
> Cheers,
> Gyula
>
> On Wed, May 1, 2024 at 9:06 AM Lajith Koova <la...@gmail.com> wrote:
>
> > Hello,
> >
> >
> > Starting discussion thread here to discuss a proposal to add Conditions
> > field in the CR status of Flink Deployment and FlinkSessionJob.
> >
> >
> > Here is the google doc with details. Please provide your thoughts/inputs.
> >
> >
> >
> >
> https://docs.google.com/document/d/12wlJCL_Vq2KZnABzK7OR7gAd1jZMmo0MxgXQXqtWODs/edit?usp=sharing
> >
> >
> > Thanks
> > Lajith
> >
>
Re: Discussion: Condition field in the CR status
Posted by Lajith Koova <la...@gmail.com>.
Thanks for the feedback Gyula. I have updated a couple of samples with
proposed conditions.
Thanks
Lajith K
On Fri, May 3, 2024 at 12:24 PM Gyula Fóra <gy...@gmail.com> wrote:
> Hi Lajith!
>
> Can you please include some examples in the document to help reviewers?
> Just some examples with the status and the proposed conditions.
>
> Cheers,
> Gyula
>
> On Wed, May 1, 2024 at 9:06 AM Lajith Koova <la...@gmail.com> wrote:
>
> > Hello,
> >
> >
> > Starting discussion thread here to discuss a proposal to add Conditions
> > field in the CR status of Flink Deployment and FlinkSessionJob.
> >
> >
> > Here is the google doc with details. Please provide your thoughts/inputs.
> >
> >
> >
> >
> https://docs.google.com/document/d/12wlJCL_Vq2KZnABzK7OR7gAd1jZMmo0MxgXQXqtWODs/edit?usp=sharing
> >
> >
> > Thanks
> > Lajith
> >
>
Re: Discussion: Condition field in the CR status
Posted by Gyula Fóra <gy...@gmail.com>.
Hi Lajith!
Can you please include some examples in the document to help reviewers?
Just some examples with the status and the proposed conditions.
Cheers,
Gyula
On Wed, May 1, 2024 at 9:06 AM Lajith Koova <la...@gmail.com> wrote:
> Hello,
>
>
> Starting discussion thread here to discuss a proposal to add Conditions
> field in the CR status of Flink Deployment and FlinkSessionJob.
>
>
> Here is the google doc with details. Please provide your thoughts/inputs.
>
>
>
> https://docs.google.com/document/d/12wlJCL_Vq2KZnABzK7OR7gAd1jZMmo0MxgXQXqtWODs/edit?usp=sharing
>
>
> Thanks
> Lajith
>