[JENKINS-72176] Split cloud management page into multiple pages #1443

scherler · 2023-10-10T19:14:05Z

https://issues.jenkins.io/browse/JENKINS-72176

I want a better experience for managing pod templates within the Clouds management screen so that it is less confusing for which pod template I am updating. Further this will help loading speed of the configuration of both pages.

Testing done

Submitter checklist

Give feedback

Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch! #1444
Edit...
Successfully updated the issue's project

There was an error updating the issue's project
Ensure that the pull request title represents the desired changelog entry

Ensure that the pull request title represents the desired changelog entry
Options
Successfully updated the issue's project

There was an error updating the issue's project
Please describe what you did

Please describe what you did
Options
Successfully updated the issue's project

There was an error updating the issue's project
Link to relevant issues in GitHub or Jira

Link to relevant issues in GitHub or Jira
Options
Successfully updated the issue's project

There was an error updating the issue's project
Link to relevant pull requests, esp. upstream and downstream changes

Link to relevant pull requests, esp. upstream and downstream changes
Options
Successfully updated the issue's project

There was an error updating the issue's project
Ensure you have provided tests - that demonstrates feature works or fixes the issue

Ensure you have provided tests - that demonstrates feature works or fixes the issue
Options
Successfully updated the issue's project

There was an error updating the issue's project
Options

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

scherler · 2023-10-11T15:44:29Z

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java

+    }
+
+    @POST
+    public HttpResponse doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, Descriptor.FormException {


that is ATM not in use anymore

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateAction.java

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

pom.xml

Vlatombe · 2023-10-12T13:18:14Z

In case you had not seen, I had filed a draft for this already in #1332

Vlatombe

Looks like it's still work in progress

src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateAction.java

src/main/resources/images/symbols/copy.svg

src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/sidepanelx.jelly

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

scherler · 2023-10-13T08:35:03Z

@Vlatombe no I did not know about your draft PR. I will try it now.

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

…esCloud.java Co-authored-by: Vincent Latombe <vincent@latombe.net>

src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java

Vlatombe · 2023-10-23T12:26:28Z

src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java

+    // TODO: Remove when JENKINS-71737 is fixed
+    @POST
+    public HttpResponse doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, Descriptor.FormException {
+
+        Jenkins j = Jenkins.get();
+        j.checkPermission(Jenkins.ADMINISTER);
+        Cloud cloud = j.getCloud(this.name);
+        if (cloud == null) {
+            return HttpResponses.notFound();
+        }
+        KubernetesCloud result = (KubernetesCloud) cloud.reconfigure(req, req.getSubmittedForm());
+        String proposedName = result.name;
+        if (!proposedName.equals(this.name)
+                && j.getCloud(proposedName) != null) {
+            throw new Descriptor.FormException(Messages.cloudAlreadyExists(proposedName), "name");
+        }
+        result.templates = this.templates;
+        j.clouds.replace(this, result);
+        j.save();
+        // take the user back to the cloud top page.
+        return FormApply.success("../..");
+    }


Even if this starts from a good intention, I think it should be split to a separate PR as it is unrelated to the intent of this PR.

src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java

Co-authored-by: Vincent Latombe <vincent@latombe.net>

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

scherler · 2023-10-23T14:38:03Z

@Vlatombe removed the workaround and applied suggestions

Vlatombe

A few last non-blocking comments

Nothing prevents duplicate pod templates (with the same name) being created. No form validation either when entering the pod template name. (nothing new, just noting)
Now that pod templates can be edited in separate pages, Pod template to inherit from could use an autocomplete component to fill in references to other pod template names

src/main/resources/org/csanchez/jenkins/plugins/kubernetes/Messages.properties

src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java

src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/sidepanel.properties

Co-authored-by: Vincent Latombe <vincent@latombe.net>

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

aneveux

Minor comments, good job!

aneveux · 2023-10-24T08:13:41Z

pom.xml

@@ -45,8 +45,7 @@
    <connectorHost />
    <jenkins.host.address />
    <slaveAgentPort />
-    <!-- TODO fix KubernetesCloudTest todos once past 2.414 -->
-    <jenkins.version>2.401.1</jenkins.version>
+    <jenkins.version>2.415</jenkins.version>


I imagine this bump is there to retrieve some necessary updates from core?

Cause it is higher than the recommended minimum version: https://www.jenkins.io/doc/developer/tutorial-improve/update-base-jenkins-version/

And also it does not align with the bom anymore (bom is using 2.401.x).

If it is intended and mandatory to have 2.415 it could be interesting to write a comment explaining it and a TODO to align the bom as soon as available?

Afair this is to get <l:delete ...>.

BOM should be bumped to the closest LTS which is 2.414.x.

The change was suggested by @Vlatombe, we need a version after jenkinsci/jenkins#7658, other than that ...

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java

src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/sidepanel.jelly

Noticed while reviewing jenkinsci/kubernetes-plugin#1443

Co-authored-by: Antoine Neveux <aneveux@cloudbees.com> Co-authored-by: Vincent Latombe <vincent@latombe.net>

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

jonesbusy · 2023-11-11T12:07:20Z

Am I the only one that since this change when updating the cloud config via the UI all pod templates are lost ?

I'm not sure if it's expected, changing cloud config should not touch pod templates since they are moved to a separate page.

I can try to reproduce it in more isolated environment or even directly with hpi:run

jglick · 2023-11-15T22:21:17Z

I can try to reproduce it in more isolated environment or even directly with hpi:run

Yes please.

jonesbusy · 2023-11-16T05:16:36Z

@jglick It's here : https://issues.jenkins.io/browse/JENKINS-72314

Can be easily reproduced with mvn hpi:run -Dport=8081 -Dhost=0.0.0.0 and not extra configuration

jglick · 2023-11-16T13:17:23Z

@Vlatombe @scherler is a ^^^ regression fix forthcoming? Otherwise this should be reverted as a hotfix.

Vlatombe · 2023-11-16T13:50:37Z

🤦 #1471

scherler · 2023-11-16T20:15:31Z

Sorry for introducing a regression and thanks for the quick fix

scherler added 15 commits October 6, 2023 11:19

[broken] wip

f9571b2

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

[BEE-30772] working listing page

be4fca7

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

[BEE-30772] WIP

2e89c57

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

[BEE-30772] Working edit of template, missing new

b7bdefe

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

[BEE-30772] remove unused imports

cee3420

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

[BEE-30772] remove unused code

b183580

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

[BEE-30772] remove unused code

558cb42

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

[BEE-30772] remove unused code

0661f6d

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

[BEE-30772] remove unused code

f1d29e6

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

[BEE-30772] remove unused code

5848de2

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

Merge branch 'master' into BEE-30772

93b833d

[BEE-30772] try the action

227712c

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

[BEE-30772] implement new template

aefb1d9

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

[BEE-30772] working delete basically

20c0acf

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

[BEE-30772] refactor so all stays in one class

2dbd016

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

scherler commented Oct 11, 2023

View reviewed changes

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java Outdated Show resolved Hide resolved

scherler commented Oct 11, 2023

View reviewed changes

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateAction.java Outdated Show resolved Hide resolved

scherler added 2 commits October 11, 2023 17:59

[BEE-30772] fix different issues

d921fc9

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

[BEE-30772] wrap up

189c5ac

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

scherler marked this pull request as ready for review October 11, 2023 20:07

scherler requested a review from a team as a code owner October 11, 2023 20:07

jtnord reviewed Oct 12, 2023

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

Vlatombe reviewed Oct 12, 2023

View reviewed changes

scherler changed the title ~~[BEE-30772]~~ JENKINS-72176] Split cloud management page into multiple pages Oct 13, 2023

[BEE-30772] fix test as pointed out by james

a011a9d

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

scherler and others added 2 commits October 13, 2023 11:00

[BEE-30772] fix delete and add security check

d31ac1f

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

Update src/main/java/org/csanchez/jenkins/plugins/kubernetes/Kubernet…

30753a8

…esCloud.java Co-authored-by: Vincent Latombe <vincent@latombe.net>

Vlatombe self-requested a review October 23, 2023 12:24

Vlatombe reviewed Oct 23, 2023

View reviewed changes

scherler and others added 3 commits October 23, 2023 15:58

Apply suggestions from code review

ea4047a

Co-authored-by: Vincent Latombe <vincent@latombe.net>

Merge branch 'master' into BEE-30772

d03c902

[BEE-30772] remove fix for core

c7180ab

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

Merge branch 'master' into BEE-30772

758d8c8

Vlatombe approved these changes Oct 24, 2023

View reviewed changes

scherler and others added 2 commits October 24, 2023 09:02

Apply suggestions from code review

2775886

Co-authored-by: Vincent Latombe <vincent@latombe.net>

[BEE-30772] removed unused

08866ec

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

aneveux approved these changes Oct 24, 2023

View reviewed changes

Vlatombe added a commit to Vlatombe/jenkins that referenced this pull request Oct 24, 2023

Wrong escaping causing incorrect message to be displayed

13f1019

Noticed while reviewing jenkinsci/kubernetes-plugin#1443

scherler and others added 2 commits October 24, 2023 10:32

Apply suggestions from code review

697f605

Co-authored-by: Antoine Neveux <aneveux@cloudbees.com> Co-authored-by: Vincent Latombe <vincent@latombe.net>

[BEE-30772] add name of the pod to confirmation

18c219c

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

Vlatombe approved these changes Oct 24, 2023

View reviewed changes

Vlatombe added the enhancement Improvements label Oct 24, 2023

Vlatombe merged commit 2d7cebd into jenkinsci:master Oct 24, 2023
6 checks passed

scherler deleted the BEE-30772 branch October 24, 2023 14:31

Vlatombe mentioned this pull request Nov 8, 2023

Separate pod templates to a new separate page #1332

Closed

8 tasks

jglick mentioned this pull request Nov 16, 2023

[JENKINS-71737] refactor to support dedicated cloud rename page #1432

Closed

Vlatombe mentioned this pull request Nov 16, 2023

[JENKINS-72314] Fix cloud save and roundtrip test #1471

Merged

timja mentioned this pull request Jan 4, 2024

Split agent templates into multiple pages jenkinsci/azure-vm-agents-plugin#493

Merged

jglick mentioned this pull request Jan 5, 2024

[fix_non_config_instance] add no-op methods for non readonly Kubernetes cloud #1491

Merged

scherler mentioned this pull request Jan 10, 2024

[templates_placeholder] allow to add templates back again #1496

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-72176] Split cloud management page into multiple pages #1443

[JENKINS-72176] Split cloud management page into multiple pages #1443

scherler commented Oct 10, 2023 •

edited

Submitter checklist

scherler Oct 11, 2023

Vlatombe commented Oct 12, 2023

Vlatombe left a comment

scherler commented Oct 13, 2023

Vlatombe Oct 23, 2023

scherler commented Oct 23, 2023

Vlatombe left a comment

aneveux left a comment

aneveux Oct 24, 2023

Vlatombe Oct 24, 2023

scherler Oct 24, 2023

jonesbusy commented Nov 11, 2023

jglick commented Nov 15, 2023

jonesbusy commented Nov 16, 2023

jglick commented Nov 16, 2023

Vlatombe commented Nov 16, 2023

scherler commented Nov 16, 2023

[JENKINS-72176] Split cloud management page into multiple pages #1443

[JENKINS-72176] Split cloud management page into multiple pages #1443

Conversation

scherler commented Oct 10, 2023 • edited

Testing done

Submitter checklist

scherler Oct 11, 2023

Choose a reason for hiding this comment

Vlatombe commented Oct 12, 2023

Vlatombe left a comment

Choose a reason for hiding this comment

scherler commented Oct 13, 2023

Vlatombe Oct 23, 2023

Choose a reason for hiding this comment

scherler commented Oct 23, 2023

Vlatombe left a comment

Choose a reason for hiding this comment

aneveux left a comment

Choose a reason for hiding this comment

aneveux Oct 24, 2023

Choose a reason for hiding this comment

Vlatombe Oct 24, 2023

Choose a reason for hiding this comment

scherler Oct 24, 2023

Choose a reason for hiding this comment

jonesbusy commented Nov 11, 2023

jglick commented Nov 15, 2023

jonesbusy commented Nov 16, 2023

jglick commented Nov 16, 2023

Vlatombe commented Nov 16, 2023

scherler commented Nov 16, 2023

scherler commented Oct 10, 2023 •

edited