Skip to content

Returns error status if asked for install action and backend detects issues#3057

Merged
mchf merged 5 commits intomasterfrom
check_issues_backend
Jan 23, 2026
Merged

Returns error status if asked for install action and backend detects issues#3057
mchf merged 5 commits intomasterfrom
check_issues_backend

Conversation

@mchf
Copy link
Copy Markdown
Contributor

@mchf mchf commented Jan 21, 2026

Problem

It is possible to start installation via CLI / API even when configuration in the backend contains issues.

Solution

Send reply with status 422 in such case

Testing

  • Tested manually

@mchf mchf marked this pull request as draft January 21, 2026 13:18
@mchf
Copy link
Copy Markdown
Contributor Author

mchf commented Jan 21, 2026

Problem

It is possible to start installation via CLI / API even when configuration in the backend contains issues.

Solution

Send reply with status 422 in such case

Testing

TODO:

  • Tested manually

@mchf mchf force-pushed the check_issues_backend branch from 366c3cf to 2c2342c Compare January 21, 2026 19:57
@coveralls
Copy link
Copy Markdown

coveralls commented Jan 21, 2026

Coverage Status

coverage: 72.998%. remained the same
when pulling 5fb7462 on check_issues_backend
into 07e213e on master.

@mchf
Copy link
Copy Markdown
Contributor Author

mchf commented Jan 22, 2026

Here is test via API:

curl -X POST \
  -H @headers.txt \
  -H "Content-Type: application/json" \
  -d '{ "install": null }' \
  -k \
  $AGAMA_URL/api/v2/action

returns:
agama_issue

However could not test it through CLI - it was for some reason unable to connect to the server.

@mchf mchf marked this pull request as ready for review January 22, 2026 07:43
@kobliha
Copy link
Copy Markdown
Contributor

kobliha commented Jan 22, 2026

Fixing https://bugzilla.suse.com/show_bug.cgi?id=1256951 (AKA Agama return code not set correctly)?

@mchf
Copy link
Copy Markdown
Contributor Author

mchf commented Jan 22, 2026

Fixing https://bugzilla.suse.com/show_bug.cgi?id=1256951 (AKA Agama return code not set correctly)?

I don't think so. This PR is about detecting whether backend sees some issues and block install action if so ... and report it back with specific http error status.

The bug on the other hand seems as invalid processing of returned errors in the CLI client. It seems, for the first read, that it always "exit 0" even when the backend cannot proceed and returns an error status.

@mchf mchf requested a review from imobachgs January 22, 2026 12:01
@mchf mchf force-pushed the check_issues_backend branch from 2c2342c to 5fb7462 Compare January 22, 2026 12:23
@mchf
Copy link
Copy Markdown
Contributor Author

mchf commented Jan 23, 2026

Copy link
Copy Markdown
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. It is up to you whether you want to use the new IsEmpty message. Perhaps we can move on and introduce that change later (when the time pressure is not so high).

/// It runs the given action.
async fn handle(&mut self, message: message::RunAction) -> Result<(), Error> {
let issues = self.issues.call(issue::message::Get).await?;
let progress = self.progress.call(progress::message::GetProgress).await?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We recently introduced a new action to check whether the progress is empty or not. See

let is_empty = self
.

Perhaps you want to give it a try (it is available in master).

@mchf mchf mentioned this pull request Jan 23, 2026
@mchf mchf merged commit d756e8f into master Jan 23, 2026
18 checks passed
@mchf mchf deleted the check_issues_backend branch January 23, 2026 09:54
imobachgs added a commit that referenced this pull request Jan 28, 2026
Fixes a few issues found during testing:

* The check to prevent starting an installation when it is not possible
was too aggressive, preventing other actions to be executed (see #3057).
* Add a line to the logs when the installation starts and when it
finishes.
* Fix the registration screen, which was loosing the "mode".
* Force to choose a mode (in case it is needed) in the product selection
screen.
mchf added a commit that referenced this pull request Feb 7, 2026
Removed pieces of old users api

## Refactoring todo as part of the cleanup
- ~~struct for ChrootCommand (kind of transparent wrapper for Command)~~
-
#3057 (review)
@imobachgs imobachgs mentioned this pull request Mar 17, 2026
imobachgs added a commit that referenced this pull request Mar 17, 2026
Prepare to release version 19.

* #1829
* #2508
* #2772
* #2818
* #2826
* #2848
* #2860
* #2863
* #2864
* #2866
* #2867
* #2869
* #2870
* #2871
* #2872
* #2873
* #2874
* #2875
* #2876
* #2877
* #2880
* #2881
* #2882
* #2884
* #2885
* #2886
* #2891
* #2892
* #2893
* #2894
* #2895
* #2896
* #2897
* #2898
* #2899
* #2900
* #2901
* #2902
* #2903
* #2904
* #2908
* #2909
* #2910
* #2912
* #2913
* #2914
* #2915
* #2916
* #2917
* #2918
* #2920
* #2921
* #2923
* #2924
* #2926
* #2928
* #2929
* #2930
* #2933
* #2934
* #2935
* #2936
* #2937
* #2938
* #2939
* #2942
* #2943
* #2944
* #2945
* #2946
* #2947
* #2948
* #2949
* #2950
* #2951
* #2952
* #2954
* #2955
* #2956
* #2957
* #2958
* #2959
* #2960
* #2961
* #2963
* #2964
* #2965
* #2967
* #2968
* #2969
* #2970
* #2971
* #2972
* #2974
* #2975
* #2977
* #2978
* #2980
* #2981
* #2982
* #2983
* #2984
* #2988
* #2989
* #2990
* #2991
* #2992
* #2993
* #2994
* #2995
* #2996
* #2997
* #2998
* #2999
* #3000
* #3001
* #3002
* #3004
* #3005
* #3006
* #3007
* #3008
* #3009
* #3011
* #3012
* #3013
* #3014
* #3015
* #3016
* #3018
* #3019
* #3020
* #3021
* #3022
* #3023
* #3024
* #3025
* #3026
* #3027
* #3028
* #3029
* #3030
* #3031
* #3033
* #3034
* #3035
* #3036
* #3037
* #3039
* #3040
* #3041
* #3042
* #3043
* #3044
* #3045
* #3046
* #3047
* #3048
* #3049
* #3050
* #3051
* #3052
* #3053
* #3054
* #3055
* #3056
* #3057
* #3058
* #3060
* #3061
* #3062
* #3063
* #3064
* #3065
* #3066
* #3067
* #3068
* #3069
* #3070
* #3071
* #3072
* #3073
* #3074
* #3075
* #3076
* #3077
* #3078
* #3079
* #3086
* #3087
* #3088
* #3089
* #3090
* #3091
* #3092
* #3093
* #3094
* #3095
* #3096
* #3097
* #3098
* #3099
* #3100
* #3101
* #3102
* #3103
* #3104
* #3105
* #3106
* #3107
* #3108
* #3109
* #3110
* #3112
* #3113
* #3114
* #3115
* #3116
* #3117
* #3118
* #3119
* #3120
* #3122
* #3123
* #3124
* #3127
* #3128
* #3129
* #3130
* #3131
* #3133
* #3134
* #3135
* #3136
* #3137
* #3138
* #3139
* #3140
* #3141
* #3142
* #3143
* #3144
* #3145
* #3146
* #3147
* #3148
* #3149
* #3150
* #3151
* #3152
* #3153
* #3154
* #3155
* #3157
* #3158
* #3159
* #3160
* #3161
* #3162
* #3163
* #3164
* #3165
* #3166
* #3167
* #3168
* #3169
* #3170
* #3174
* #3175
* #3176
* #3177
* #3178
* #3179
* #3181
* #3182
* #3184
* #3185
* #3186
* #3188
* #3189
* #3190
* #3191
* #3192
* #3194
* #3195
* #3196
* #3197
* #3198
* #3199
* #3200
* #3201
* #3202
* #3203
* #3205
* #3206
* #3208
* #3209
* #3210
* #3213
* #3214
* #3215
* #3216
* #3217
* #3218
* #3219
* #3220
* #3222
* #3223
* #3224
* #3225
* #3226
* #3227
* #3228
* #3229
* #3230
* #3231
* #3232
* #3233
* #3234
* #3235
* #3236
* #3237
* #3238
* #3239
* #3240
* #3241
* #3242
* #3243
* #3244
* #3246
* #3247
* #3248
* #3250
* #3251
* #3252
* #3253
* #3254
* #3255
* #3256
* #3257
* #3258
* #3259
* #3260
* #3261
* #3262
* #3263
* #3265
* #3266
* #3267
* #3268
* #3269
* #3270
* #3271
* #3272
* #3273
* #3274
* #3275
* #3276
* #3277
* #3278
* #3279
* #3280
* #3281
* #3282
* #3283
* #3284
* #3285
* #3286
* #3287
* #3288
* #3289
* #3290
* #3291
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants