Issue #517: PetSet and DaemonSet support #620

ucosty · 2016-10-30T18:11:03Z

I've had some time to work on issue #517, adding support for generating PetSet and DaemonSet configurations using the fabric8-maven-plugin. I'll have a go at updating the documentation as well, to reflect some of these changes.

For now I've been using it by configuring the fmp-controller enricher to generate the PetSet and DaemonSet types.

The following snippet of code seems to work for my needs.

<enricher>
    <config>
        <fmp-controller>
            <type>petSet</type>
        </fmp-controller>
    </config>
</enricher>

rhuss · 2016-11-11T16:59:54Z

Thanks, will have a look ASAP.

rhuss

Except for some minor things, looks good to me.

Could you please fix that and update the PR ?

Thanks a lot !

rhuss · 2016-11-11T17:16:00Z

core/src/main/java/io/fabric8/maven/core/handler/PetSetHandler.java

+
+    private ObjectMeta createPetSetMetaData(ResourceConfig config) {
+        return new ObjectMetaBuilder()
+                .withName(KubernetesHelper.validateKubernetesId(config.getReplicaSetName(), "replica set name"))


wdyt, maybe we should rename replicaSetName to controllerName ?

Good idea, I've added that to this PR.

rhuss · 2016-11-11T17:22:13Z

core/src/main/java/io/fabric8/maven/core/handler/ProbeHandler.java

            probe.setTimeoutSeconds(timeoutSeconds);
        }
        HTTPGetAction getAction = getHTTPGetAction(probeConfig.getGetUrl());
+        if(StringUtils.isEmpty(getAction.getHost())) {


This check can lead to a NPE since getAction can be null. Could explain briefly why an empty String as host is an issue ?

It was a bit of a hack to allow an empty HTTP host to be set in the liveness/readiness checks. Without this minor change an empty host value would result in a literal empty host value being added to the output YAML. For now I've just removed this, as it should have been a part of issue #622.

rhuss · 2016-11-11T17:25:24Z

plugin/src/main/java/io/fabric8/maven/plugin/enricher/MetadataVisitor.java

+
+        @Override
+        protected Kind getKind() {
+            return Kind.DAEMON_SET;


Shouldn't this be Kind.PET_SET ?

Indeed it should be, fixed.

oscerd · 2016-12-11T09:22:08Z

Any news on this one? :-)

rhuss

Thanks a lot and sorry for the long delay. There is only one minor issue, if this is fixed we can merge it into master.

rhuss · 2016-12-12T07:33:28Z

plugin/src/main/java/io/fabric8/maven/plugin/enricher/SelectorVisitor.java

+        @Override
+        public void visit(PetSetSpecBuilder item) {
+            Map<String, String> selectorMatchLabels =
+                    KubernetesResourceUtil.removeVersionSelector(enricherManager.extractSelector(getConfig(), Kind.REPLICATION_CONTROLLER));


Should be Kind.PET_SET

rhuss · 2016-12-12T07:35:04Z

plugin/src/main/java/io/fabric8/maven/plugin/enricher/SelectorVisitor.java

+        @Override
+        public void visit(DaemonSetSpecBuilder item) {
+            Map<String, String> selectorMatchLabels =
+                    KubernetesResourceUtil.removeVersionSelector(enricherManager.extractSelector(getConfig(), Kind.REPLICATION_CONTROLLER));


shoud be Kind.DAEMON_SET (btw, there is an error for the Deployment selector, I will fix that.

rhuss · 2016-12-17T08:25:55Z

If you don't mind I would fix the outstanding issues and integrate this PR next week.

oscerd · 2017-01-04T12:25:28Z

I'd like to see this one merged, to update the cassandra quickstart with a PetSet configuration :-)

rhuss · 2017-01-04T13:56:54Z

@oscerd back right now, will go through the PRs here soon. Happy new year ;-) !

oscerd · 2017-01-05T07:15:12Z

@rhuss happy new year! :-)

Thanks for your work! :-)

ucosty · 2017-02-03T16:12:18Z

Sorry for the delay, this has been updated now.

fixed up changed method names

…host field for probe check to leave the host field out of the check as per Kubernetes documentation

…r pet sets

codecov · 2017-02-03T16:42:36Z

Codecov Report

Merging #620 into master will decrease coverage by -0.78%.

@@             Coverage Diff              @@
##             master     #620      +/-   ##
============================================
- Coverage     17.02%   16.24%   -0.78%     
+ Complexity      431      403      -28     
============================================
  Files           150      148       -2     
  Lines          8102     8102              
  Branches       1478     1468      -10     
============================================
- Hits           1379     1316      -63     
- Misses         6565     6633      +68     
+ Partials        158      153       -5

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1dc2abd...e93c69e. Read the comment docs.

crashdump · 2017-02-07T11:09:12Z

I would love to see this one merged.

rhuss · 2017-02-07T11:25:40Z

will check that this later today or tomorrow, will merge it this week.

jstrachan · 2017-02-07T11:30:01Z

though PetSet is now StatefulSet right?

rhuss · 2017-02-07T11:31:38Z

Right, should be changed to StatefulSet.

rhuss · 2017-02-07T11:32:36Z

We could support PetSet for backwards compatibility, too, but probably its better to start with out baggage. So I opts for supporting only StatefulSet.

rawlingsj · 2017-02-07T11:34:07Z

FWIW this PR will bring in the StatefulSet rename fabric8io/kubernetes-client#646, once we're happy with that I was going to upgrade fabric8 and then fabric8-maven-plugin asap.

rhuss · 2017-02-09T17:01:26Z

@rawlingsj thanks a lot for StatefulSet, I'm going to rename it in this PR. Is the kubernetes-client already released with this addition ?

rawlingsj · 2017-02-09T17:03:43Z

thanks a lot for StatefulSet, I'm going to rename it in this PR. Is the kubernetes-client already released with this addition ?

no problem! yes if you rebase this PR then you should get the new fabric8 version which includes the updated kubernetes client and model with StatefulSet.

rhuss · 2017-02-09T17:06:10Z

Hmm. I see still fabric8 2.2.193 as the latest referring to kubernetes-client 2.0.0 (but 2.0.2 is the latest which contains the changes).

rhuss · 2017-02-09T17:08:34Z

Let me debug that a bit here, looks a bit weird ... (using a kubernetes client 1.4.33 and so). Probably my fault.

rhuss · 2017-02-09T17:09:56Z

ok, got it now properly. all good :)

rawlingsj · 2017-02-09T17:10:24Z

Looks like there's been a couple more releases but 2.0.0 was the one that I did which includes the statefulset rename. So hopefully you should be good.

rawlingsj · 2017-02-09T17:10:55Z

ok, got it now properly. all good :)

ah cool :)

rhuss · 2017-02-09T18:53:08Z

I updated from PetSet to StatefulSet (thanks to @rawlingsj for the upstream kubernetes-client update !).

Changes look good to me. Some tests would have been nice but due to the fact that we don't have tests for the other handlers, too, its not fair to call for one here now ;-)

rhuss · 2017-02-09T18:58:03Z

[merge]

ucosty changed the title ~~PetSet and DaemonSet support~~ Issue #517PetSet and DaemonSet support Oct 30, 2016

ucosty changed the title ~~Issue #517PetSet and DaemonSet support~~ Issue #517: PetSet and DaemonSet support Oct 30, 2016

rhuss suggested changes Nov 11, 2016

View reviewed changes

rhuss suggested changes Dec 12, 2016

View reviewed changes

rhuss added Feedback required pr/change-requested Reviewer requested a change and removed Feedback required labels Dec 12, 2016

ucosty and others added 9 commits February 3, 2017 16:20

Added support for PetSet and DaemonSet controller types

067792d

fixed up changed method names

Fixed up dependency regression due to libary rollback, allowed empty …

eea54ba

…host field for probe check to leave the host field out of the check as per Kubernetes documentation

Update the Fabric8 module dependency to version 2.2.173

50ac05f

Fixed up some PetSet and DaemonSet selector support

30ced8b

Rename replica set name to controller name, fixed up metadata type fo…

9ae4db5

…r pet sets

removed probe changes, will rework into issue 622

a3728b2

Fixed selector kinds

7256d8f

Treat petset and daemonset like deployments

a1b3f0e

Fixed typo

3ec77b6

ucosty force-pushed the petset_daemonset_support branch from 9262620 to 3ec77b6 Compare February 3, 2017 16:38

rhuss removed the pr/change-requested Reviewer requested a change label Feb 9, 2017

Merge branch 'master' into petset_daemonset_support

16f73e7

Changed from "PetSet" to "StatefulSet"

e93c69e

rhuss approved these changes Feb 9, 2017

View reviewed changes

fusesource-ci merged commit bf56cf2 into fabric8io:master Feb 9, 2017

Issue #517: PetSet and DaemonSet support #620

Issue #517: PetSet and DaemonSet support #620

Uh oh!

Conversation

ucosty commented Oct 30, 2016

Uh oh!

rhuss commented Nov 11, 2016

Uh oh!

rhuss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oscerd commented Dec 11, 2016

Uh oh!

rhuss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhuss commented Dec 17, 2016

Uh oh!

oscerd commented Jan 4, 2017

Uh oh!

rhuss commented Jan 4, 2017

Uh oh!

oscerd commented Jan 5, 2017

Uh oh!

ucosty commented Feb 3, 2017

Uh oh!

codecov bot commented Feb 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

crashdump commented Feb 7, 2017

Uh oh!

rhuss commented Feb 7, 2017

Uh oh!

jstrachan commented Feb 7, 2017

Uh oh!

rhuss commented Feb 7, 2017

Uh oh!

rhuss commented Feb 7, 2017

Uh oh!

rawlingsj commented Feb 7, 2017

Uh oh!

rhuss commented Feb 9, 2017

Uh oh!

rawlingsj commented Feb 9, 2017

Uh oh!

rhuss commented Feb 9, 2017

Uh oh!

rhuss commented Feb 9, 2017

Uh oh!

rhuss commented Feb 9, 2017

Uh oh!

rawlingsj commented Feb 9, 2017

Uh oh!

rawlingsj commented Feb 9, 2017

Uh oh!

rhuss commented Feb 9, 2017

Uh oh!

rhuss commented Feb 9, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

codecov bot commented Feb 3, 2017 •

edited

Loading