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.

Sign up for GitHub

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

Jump to bottom

[bug] app restart should be synchronous #3036

Closed
danail-branekov opened this issue Dec 8, 2023 · 0 comments
Closed

[bug] app restart should be synchronous #3036

danail-branekov opened this issue Dec 8, 2023 · 0 comments
Labels
bug Something isn't working

Comments

@danail-branekov
Copy link
Member

danail-branekov commented Dec 8, 2023 •
edited
Loading

According to cf api docs on app restart,

This endpoint will synchronously stop and start an application. Unlike the start and stop actions, this endpoint will error if the app is not successfully stopped in the runtime.

In Korifi, however, app restart is currently implemented by setting the desired state to stopped and then back to started without actually waiting for the app go down and then up.

This bug is most probably exposed by this flake

130 ❯❯ flake-hunter "\[It\] mounts the credentials as files into the container"
+-------+----------------------------------+-----------------------------------------------------
| Ended | Job                              | Url
+-------+----------------------------------+-----------------------------------------------------
| 4h    | main/run-e2es-periodic           | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-periodic/builds/12528
| 2d    | main/run-e2es-pr                 | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-pr/builds/2350
| 31d   | main/run-e2es-eks-pr             | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-pr/builds/1296
| 53d   | main/run-e2es-periodic           | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-periodic/builds/11550
| 75d   | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/3979
| 81d   | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/3911
| 87d   | main/run-e2es-pr                 | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-pr/builds/2131
| 89d   | main/run-e2es-periodic           | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-periodic/builds/10881
| 90d   | main/run-e2es-eks-pr             | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-pr/builds/1141
| 95d   | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/3690
| 101d  | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/3582
| 103d  | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/3537
| 105d  | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/3488
| 108d  | main/run-e2es-eks-pr             | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-pr/builds/1095
| 111d  | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/3387
| 114d  | main/run-e2es-eks-pr             | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-pr/builds/1052
| 115d  | main/run-e2es-pr                 | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-pr/builds/2024.1
| 115d  | main/run-e2es-pr                 | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-pr/builds/2024
| 116d  | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/3301
| 117d  | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/3284
| 117d  | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/3281
| 121d  | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/3198
| 121d  | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/3194
| 122d  | main/run-e2es-eks                | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks/builds/444
| 122d  | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/3185
| 122d  | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/3181
| 124d  | main/run-e2es-periodic           | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-periodic/builds/10259
| 131d  | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/2940
| 167d  | main/run-e2es-eks-periodic       | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-periodic/builds/2065
| 258d  | main/run-e2es-eks-pr             | https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-e2es-eks-pr/builds/467
+-------+----------------------------------+-----------------------------------------------------
@danail-branekov danail-branekov changed the title [bug] app restart is asynchronous [bug] app restart should be synchronous Dec 8, 2023
@danail-branekov danail-branekov added the bug Something isn't working label Dec 8, 2023
danail-branekov added a commit that referenced this issue Apr 5, 2024
@danail-branekov @georgethebeatle
App restart is synchronous
8a19605 
* Introduce `CFApp.Status.ActualState` to reflect the actual state of
  the app. The start/stop/restart operations waits for that field to
  become equal to the desired app state
* Introduce `CFProcess.Status.ActualInstances` to reflect the actual app
  workload instances
* Introduce `AppWorkload.Status.ActualInstances` to reflect the actual
  statefulset replicas
* The state chain is tested by a test in the `crds` suite
* The existing crd test is deleted as it has become obsolete by the test
  above

fixes #3036

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
danail-branekov added a commit that referenced this issue Apr 5, 2024
@danail-branekov @georgethebeatle
App restart is synchronous
439a0c2 
* Introduce `CFApp.Status.ActualState` to reflect the actual state of
  the app. The start/stop/restart operations waits for that field to
  become equal to the desired app state
* Introduce `CFProcess.Status.ActualInstances` to reflect the actual app
  workload instances
* Introduce `AppWorkload.Status.ActualInstances` to reflect the actual
  statefulset replicas
* The state chain is tested by a test in the `crds` suite
* The existing crd test is deleted as it has become obsolete by the test
  above

fixes #3036

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
danail-branekov added a commit that referenced this issue Apr 5, 2024
@danail-branekov @georgethebeatle
App restart is synchronous
5beea86 
* Introduce `CFApp.Status.ActualState` to reflect the actual state of
  the app. The start/stop/restart operations waits for that field to
  become equal to the desired app state
* Introduce `CFProcess.Status.ActualInstances` to reflect the actual app
  workload instances
* Introduce `AppWorkload.Status.ActualInstances` to reflect the actual
  statefulset replicas
* The state chain is tested by a test in the `crds` suite
* The existing crd test is deleted as it has become obsolete by the test
  above

fixes #3036

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
danail-branekov added a commit that referenced this issue Apr 8, 2024
@danail-branekov @georgethebeatle
App restart is synchronous
8b6823c 
* Introduce `CFApp.Status.ActualState` to reflect the actual state of
  the app. The start/stop/restart operations waits for that field to
  become equal to the desired app state
* Introduce `CFProcess.Status.ActualInstances` to reflect the actual app
  workload instances
* Introduce `AppWorkload.Status.ActualInstances` to reflect the actual
  statefulset replicas
* The state chain is tested by a test in the `crds` suite
* The existing crd test is deleted as it has become obsolete by the test
  above

fixes #3036

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
danail-branekov added a commit that referenced this issue Apr 8, 2024
@danail-branekov @georgethebeatle
App restart is synchronous
5c267b5 
* Introduce `CFApp.Status.ActualState` to reflect the actual state of
  the app. The start/stop/restart operations waits for that field to
  become equal to the desired app state
* Introduce `CFProcess.Status.ActualInstances` to reflect the actual app
  workload instances
* Introduce `AppWorkload.Status.ActualInstances` to reflect the actual
  statefulset replicas
* The state chain is tested by a test in the `crds` suite
* The existing crd test is deleted as it has become obsolete by the test
  above

fixes #3036

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
@georgethebeatle georgethebeatle mentioned this issue Apr 8, 2024
Merged
danail-branekov added a commit that referenced this issue Apr 8, 2024
@danail-branekov @georgethebeatle
App restart is synchronous
209f657 
* Introduce `CFApp.Status.ActualState` to reflect the actual state of
  the app. The start/stop/restart operations waits for that field to
  become equal to the desired app state
* Introduce `CFProcess.Status.ActualInstances` to reflect the actual app
  workload instances
* Introduce `AppWorkload.Status.ActualInstances` to reflect the actual
  statefulset replicas
* The state chain is tested by a test in the `crds` suite
* The existing crd test is deleted as it has become obsolete by the test
  above

fixes #3036

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
danail-branekov added a commit that referenced this issue Apr 9, 2024
@danail-branekov @georgethebeatle
App restart is synchronous
6fc295a 
* Introduce `CFApp.Status.ActualState` to reflect the actual state of
  the app. The start/stop/restart operations waits for that field to
  become equal to the desired app state
* Introduce `CFProcess.Status.ActualInstances` to reflect the actual app
  workload instances
* Introduce `AppWorkload.Status.ActualInstances` to reflect the actual
  statefulset replicas
* The state chain is tested by a test in the `crds` suite
* The existing crd test is deleted as it has become obsolete by the test
  above

fixes #3036

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
georgethebeatle added a commit that referenced this issue Apr 9, 2024
@danail-branekov @georgethebeatle
App restart is synchronous
0b2be7e 
* Introduce `CFApp.Status.ActualState` to reflect the actual state of
  the app. The start/stop/restart operations waits for that field to
  become equal to the desired app state
* Introduce `CFProcess.Status.ActualInstances` to reflect the actual app
  workload instances
* Introduce `AppWorkload.Status.ActualInstances` to reflect the actual
  statefulset replicas
* The state chain is tested by a test in the `crds` suite
* The existing crd test is deleted as it has become obsolete by the test
  above

fixes #3036

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
georgethebeatle added a commit that referenced this issue Apr 9, 2024
@danail-branekov @georgethebeatle
App restart is synchronous
97615f3 
* Introduce `CFApp.Status.ActualState` to reflect the actual state of
  the app. The start/stop/restart operations waits for that field to
  become equal to the desired app state
* Introduce `CFProcess.Status.ActualInstances` to reflect the actual app
  workload instances
* Introduce `AppWorkload.Status.ActualInstances` to reflect the actual
  statefulset replicas
* The state chain is tested by a test in the `crds` suite
* The existing crd test is deleted as it has become obsolete by the test
  above

fixes #3036

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
georgethebeatle added a commit that referenced this issue Apr 9, 2024
@danail-branekov @georgethebeatle
App restart is synchronous
c7f56d7 
* Introduce `CFApp.Status.ActualState` to reflect the actual state of
  the app. The start/stop/restart operations waits for that field to
  become equal to the desired app state
* Introduce `CFProcess.Status.ActualInstances` to reflect the actual app
  workload instances
* Introduce `AppWorkload.Status.ActualInstances` to reflect the actual
  statefulset replicas
* The state chain is tested by a test in the `crds` suite
* The existing crd test is deleted as it has become obsolete by the test
  above

fixes #3036

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
georgethebeatle added a commit that referenced this issue Apr 9, 2024
@danail-branekov @georgethebeatle
App restart is synchronous
97df2d1 
* Introduce `CFApp.Status.ActualState` to reflect the actual state of
  the app. The start/stop/restart operations waits for that field to
  become equal to the desired app state
* Introduce `CFProcess.Status.ActualInstances` to reflect the actual app
  workload instances
* Introduce `AppWorkload.Status.ActualInstances` to reflect the actual
  statefulset replicas
* The state chain is tested by a test in the `crds` suite
* The existing crd test is deleted as it has become obsolete by the test
  above

fixes #3036

Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
@georgethebeatle georgethebeatle mentioned this issue Apr 17, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
Korifi - Backlog
Status: ✅ Done
Milestone
No milestone
Development

No branches or pull requests
1 participant
@danail-branekov