DSCI CRD: allow default as a value for logmode

[ykaliuta]

default is a separate mode in the sources, not just selection of one of the others, so allow it explicitly.

There is no difference if it is set to "" explicitly or not set since "" is a string default value.

In the current code as soon as devFlags are present, the mode is taken in use, so it is reset to default even if it is not set which does not look right (I would expect leave it untouched if is not set). If it is fixed, then it will be impossible to reset logging mode to default from devel or prod: if "" is considered as "not set explicitly", then there is no value for default, the object checker rejects values not in list.

Description

How Has This Been Tested?

Screenshot or short clip

Merge criteria

• You have read the contributors guide.
• Commit messages are meaningful - have a clear and concise summary and detailed explanation of what was changed and why.
• Pull Request contains a description of the solution, a link to the JIRA issue, and to any dependent or related Pull Request.
• Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
• The developer has manually tested the changes and verified that the changes work

[ykaliuta]

/cc @zdtsw

[zdtsw]

i am fine with this change.
but try to understand the case better:
in DSCI under devFlags we currently have

1. logmode
2. manifestsUri => this should have been set to deprecated (docs(api): set manifestUri as deprecated #1137)
   I understand : " In the current code as soon as devFlags are present, the mode is taken in use" as

spec:
  devFlags:
    logmode: ""
    manifestsUri:  ....

is the same as

spec:
  applicationsNamespace: opendatahub
  devFlags:
       manifestsUri:  ....

because "" is the default value for string

but since we do not use manifestsUri
so it can only be done explictiliy as

spec:
  devFlags:
    logmode: ""

not

spec:
  devFlags:

(as this will remove the whole devFlags)

is this the concern for the PR?

[ykaliuta]

is this the concern for the PR?

May be not exactly. The point is as soon as devFlags present, logmode is considered to be set to "" even if it is not. I actually had in mind scenario where a "user" sets just manifests (ok, deprecated now, but anyway) and automatically resets the logmode.

[ykaliuta]

is this the concern for the PR?

May be not exactly. The point is as soon as devFlags present, logmode is considered to be set to "" even if it is not. I actually had in mind scenario where a "user" sets just manifests (ok, deprecated now, but anyway) and automatically resets the logmode.

And if it fixed (I can only fix it with ignoring of ""), there is no way to force default anymore

[zdtsw]

is this the concern for the PR?

May be not exactly. The point is as soon as devFlags present, logmode is considered to be set to "" even if it is not. I actually had in mind scenario where a "user" sets just manifests (ok, deprecated now, but anyway) and automatically resets the logmode.

yep, so if the user want to set logmode to $value or "" i dont think it could be a case that we have devFlags without logmode there.
but yeah, as i said, i dont see problem with having a value default defined there.

[ykaliuta]

i dont think it could be a case that we have devFlags without logmode there

Yep. That is the hypothetical case I'm addressing

[zdtsw]

would like to have 2nd eyes on this as well

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zdtsw

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [zdtsw]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment