Add more states to info message

[zedi-pramodh]

Add PENDING, APPFAILED and SCHEDULING states to info message
Need to use APPFAILED since FAILED state is already taken by BaseOsStatus

[zedi-pramodh]

I am not clear on the Yetus errors. Do we need to prefix that states with Z_SW_STATE_ ?

[eriknordmark]

I am not clear on the Yetus errors. Do we need to prefix that states with Z_SW_STATE_ ?

Yes, if this was a completely new enum we should do that. But when adding new enum values to an existing enum we follow the existing style (and ignore such yetus errors).

[eriknordmark]

The fact that FAILED exists and is used in some object for BaseOS doesn't mean it can't be used in other objects such as the app instance. So I don't think you need to add a uniq APPFAILED value here.

[zedi-pramodh]

But the make proto compilation failed. That's why I had to change.

bash-3.2$ make proto
rm -rf go//; mkdir -p go # building proto-api-go
protoc -I./proto --go_out=paths=source_relative:./go proto//*.proto
info/info.proto:860:3: "FAILED" is already defined in "org.lfedge.eve.info".
info/info.proto:860:3: Note that enum values use C++ scoping rules, meaning that enum values are siblings of their type, not children of it. Therefore, "FAILED" must be unique within "org.lfedge.eve.info", not just within "BaseOsStatus".

[eriknordmark]

Ah - I missed that it was in another enum definition.

Do you really need a *FAILED or can you use the existing ERROR enum value?

[zedi-pramodh]

FAILED is returned by kubernetes as one the states for the app. We better be explicit and return same error. I looked at other options and only thing that comes close is HALTED, which does not convey the failed state.

[eriknordmark]

Why can't you use the existing ERROR?
When used on an app instance there is the associated
repeated ErrorInfo appErr = 14;
which enables you to specify additional details as to the nature/reason for the error.

[zedi-pramodh]

IF you have a sample PR I look into that ?

[zedi-pramodh]

I looked at the code deeply. repeated ErrorInfo appErr = 14; is the detailed error description for an error state.
I am creating new error states, so not clear to me on how to proceed here.

[eriknordmark]

I looked at the code deeply. repeated ErrorInfo appErr = 14; is the detailed error description for an error state. I am creating new error states, so not clear to me on how to proceed here.

We map the pillar strate to the API state in https://github.com/lf-edge/eve/blob/d72bdf814fc59d8519ccb0c9fec2981664c310e9/pkg/pillar/types/types.go#L111

(You need to extend that function in any case to avoid the log.Fatal).

There you can map FAILED to ERROR.
If you want to set the error info you need to do that in the caller in https://github.com/lf-edge/eve/blob/d72bdf814fc59d8519ccb0c9fec2981664c310e9/pkg/pillar/cmd/zedagent/handlemetrics.go#L1061

[zedi-pramodh]

Yes I extended the types.go function to handle fatal errors. Ok then I will removed APPFAILED from this PR and map FAILED to ERROR in types.go. That should address your questions I guess.

[zedi-pramodh]

Force updated the commits

[eriknordmark]

It's better to ignore the yetus complaint about the prefix to make all of the values in a given enum follow the same style. So PENDING and SCHEDULING makes more sense as names.

[zedi-pramodh]

OK I thought you told me to add the prefix for new enums. I will change that and resubmit. BTW, I did publish the new make proto files too. Not sure why it is complaining.

[eriknordmark]

I addition to the names, you also need to drop and regenerate the svg file since it shows as a conflict (images/devconfig.svg).

[zedi-pramodh]

I regenerated "make proto" files. Not sure why it still shows conflict.

[zedi-pramodh]

I know why, I am not on latest main. I will rebase to main and retry.

[zedi-pramodh]

ok its clean now.

[eriknordmark]

LGTM. Ignoring the buflint complains to follow the style of the other values in this existing enum.