Rollout retry #1454

strailov · 2023-10-17T08:26:59Z

No description provided.

Signed-off-by: Stanislav Trailov <Stanislav.Trailov@bosch.io>

hawkbit-bot · 2023-10-17T08:27:01Z

Can one of the admins verify this patch?

...tory/hawkbit-repository-core/src/main/java/org/eclipse/hawkbit/repository/RolloutHelper.java

avgustinmm · 2023-10-17T17:01:48Z

...it-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java

@@ -730,4 +752,38 @@ public void triggerNextGroup(final long rolloutId) {
        startNextRolloutGroupAction.exec(rollout, latestRunning);
    }

+    private TargetCount calculateTargetsAndFilterBasedOnRetriedRollout(final String targetFilter, final Long createdAt, final Long dsTypeId) {


improper name = could be not based on retried

Don't see need to have different method if it adds new type TargetCount (again not correctly reflects the class content)

Different method was needed in order to escape duplication ...

avgustinmm · 2023-10-17T17:04:17Z

...it-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java

+        return new TargetCount(totalTargets, baseFilter);
+    }
+
+    private static class TargetCount {


could be record

avgustinmm · 2023-10-17T17:10:31Z

...it-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtRolloutMapper.java

@@ -132,6 +132,18 @@ static RolloutCreate fromRequest(final EntityFactory entityFactory, final MgmtRo
                .weight(restRequest.getWeight());
    }

+    static RolloutCreate fromRetriedRollout(final EntityFactory entityFactory, final Rollout rollout) {
+        return entityFactory.rollout().create()
+                .name(rollout.getName().concat("_retried"))


better "_retry"

avgustinmm · 2023-10-17T17:12:52Z

...bit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetManagement.java

+            TargetSpecifications.isCompatibleWithDistributionSetType(dsTypeId));
+
+        if (createdAt != null) {
+            specList.add(TargetSpecifications.createdBefore(createdAt));


why createdBefore is used - rollout fail filter would not contain devices created after anyway

avgustinmm · 2023-10-17T17:17:58Z

...tory/hawkbit-repository-core/src/main/java/org/eclipse/hawkbit/repository/RolloutHelper.java

@@ -253,4 +256,12 @@ public static void checkIfRolloutCanStarted(final Rollout rollout, final Rollout
                    + rollout.getStatus().name().toLowerCase());
        }
    }
+
+    public static boolean isRolloutRetried(final String targetFilter) {
+        return targetFilter.contains("failedRollout");


is this case sensitive?
aren't lower cased attributes are supported at the moment? e.g. "createdat"
if search attributes are case insensitive this check shall be also such.
If not - better use lowercase as in other cases

Signed-off-by: Stanislav Trailov <Stanislav.Trailov@bosch.io>

avgustinmm · 2023-10-18T10:51:20Z

...it-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutManagement.java

@@ -616,12 +626,21 @@ public void cancelRolloutsForDistributionSet(final DistributionSet set) {
    }

    private RolloutGroupsValidation validateTargetsInGroups(final List<RolloutGroup> groups, final String baseFilter,
-            final long totalTargets, final Long dsTypeId) {
+            final long totalTargets, final Long dsTypeId, final Long createdAt) {


createdAt is not really used anymore

avgustinmm · 2023-10-18T10:53:06Z

...pa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/TargetSpecifications.java

+        };
+    }
+
+    public static Specification<JpaTarget> createdBefore(final Long time) {


Is this specification used?

avgustinmm · 2023-10-18T10:53:33Z

...ry/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TargetManagement.java

+     * @param dsTypeId
+     *            ID of the {@link DistributionSetType} the targets need to be
+     *            compatible with
+     * @param createdAt


not a parameter anymore

avgustinmm · 2023-10-18T11:04:08Z

...bit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetManagement.java

+    public long countByFailedInRolloutAndCompatible(final String rolloutId, final Long dsTypeId) {
+        final List<Specification<JpaTarget>> specList = Arrays.asList(
+            TargetSpecifications.failedActionsForRollout(rolloutId),
+            TargetSpecifications.isCompatibleWithDistributionSetType(dsTypeId));


aren't the failed targets already checked for compatibility?

avgustinmm · 2023-10-18T11:05:00Z

...bit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetManagement.java

+        final List<Specification<JpaTarget>> specList = Arrays.asList(
+            TargetSpecifications.failedActionsForRollout(rolloutId),
+            TargetSpecifications.isNotInRolloutGroups(groups),
+            TargetSpecifications.isCompatibleWithDistributionSetType(distributionSetType.getId()));


aren't the failed targets already checked for compatibility?

avgustinmm · 2023-10-18T11:05:11Z

...bit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetManagement.java

+        final List<Specification<JpaTarget>> specList = Arrays.asList(
+            TargetSpecifications.failedActionsForRollout(rolloutId),
+            TargetSpecifications.isNotInRolloutGroups(groups),
+            TargetSpecifications.isCompatibleWithDistributionSetType(distributionSetType.getId())


aren't the failed targets already checked for compatibility?

Signed-off-by: Stanislav Trailov <Stanislav.Trailov@bosch.io>

avgustinmm · 2023-10-19T05:43:22Z

...ry/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TargetManagement.java

+     *            the list of {@link RolloutGroup}s
+     * @param rolloutId
+     *            rolloutId of the rollout to be retried.
+     * @param distributionSetType


not a parameter anymore

avgustinmm · 2023-10-19T05:43:40Z

...ry/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TargetManagement.java

+     *            the list of {@link RolloutGroup}s
+     * @param rolloutId
+     *            rolloutId of the rollout to be retried.
+     * @param distributionSetType


not parameter anymore

Signed-off-by: Stanislav Trailov <Stanislav.Trailov@bosch.io>

avgustinmm · 2023-10-19T06:36:35Z

lgtm

strailov added 3 commits October 17, 2023 11:19

Rollout retry mechanism initial commit

c49af0c

Signed-off-by: Stanislav Trailov <Stanislav.Trailov@bosch.io>

Remove test target fields for filter query

6c10071

Signed-off-by: Stanislav Trailov <Stanislav.Trailov@bosch.io>

minor refactoring

f049475

Signed-off-by: Stanislav Trailov <Stanislav.Trailov@bosch.io>

avgustinmm requested changes Oct 17, 2023

View reviewed changes

Fixes after review

b728b12

Signed-off-by: Stanislav Trailov <Stanislav.Trailov@bosch.io>

avgustinmm requested changes Oct 18, 2023

View reviewed changes

strailov added 2 commits October 18, 2023 15:16

more refactoring after review

2041cd7

Signed-off-by: Stanislav Trailov <Stanislav.Trailov@bosch.io>

skip compatibility check of dstype for retried rollout

ce35d53

Signed-off-by: Stanislav Trailov <Stanislav.Trailov@bosch.io>

avgustinmm requested changes Oct 19, 2023

View reviewed changes

remove dsType from javadoc

b0ed733

Signed-off-by: Stanislav Trailov <Stanislav.Trailov@bosch.io>

avgustinmm approved these changes Oct 19, 2023

View reviewed changes

strailov merged commit 44e7a72 into eclipse:master Oct 19, 2023
2 checks passed

strailov deleted the feature/retry_rollout branch October 19, 2023 06:58

avgustinmm added this to the 0.3.0 milestone Nov 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rollout retry #1454

Rollout retry #1454

strailov commented Oct 17, 2023

hawkbit-bot commented Oct 17, 2023

avgustinmm Oct 17, 2023

avgustinmm Oct 17, 2023

strailov Oct 18, 2023

avgustinmm Oct 17, 2023

avgustinmm Oct 17, 2023

avgustinmm Oct 17, 2023

avgustinmm Oct 17, 2023

avgustinmm Oct 18, 2023

avgustinmm Oct 18, 2023

avgustinmm Oct 18, 2023

avgustinmm Oct 18, 2023

avgustinmm Oct 18, 2023

avgustinmm Oct 18, 2023

avgustinmm Oct 19, 2023

avgustinmm Oct 19, 2023

avgustinmm commented Oct 19, 2023

Rollout retry #1454

Rollout retry #1454

Conversation

strailov commented Oct 17, 2023

hawkbit-bot commented Oct 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avgustinmm commented Oct 19, 2023