Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

Introduce etcd petset #1295

Closed

Conversation

ingvagabund
Copy link
Contributor

@ingvagabund ingvagabund commented Jun 30, 2016

Example of etcd cluster via petsets


This change is Reviewable

@ncdc
Copy link

ncdc commented Jun 30, 2016

@bprashanth @smarterclayton PTAL

done

# re-join member
ETCDCTL_ENDPOINT=${EPS} etcdctl member update ${member_id} http://${HOSTNAME}.${PETSET_NAME}:2380

Choose a reason for hiding this comment

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

it would be great if you could do this in an init container and write the config to some hostPath shared location that etcd reads on startup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can't. The /var/run/etcd/meta is generated after the cluster is bootstrapped. The file contains a member id which is know after the cluster started.

@bprashanth
Copy link

Seems like there're a lot of similarities with this and the zookeeper setup, would be nice to duplicate the pattern if possible (https://github.com/kubernetes/contrib/tree/master/pets/zookeeper). It would also make scaling easier.

@tbg
Copy link

tbg commented Jul 3, 2016

I tried running this for a while to no avail (all on 1.3) until I realized that I needed to manually create the PVCs expected by the cluster ahead of time. Is that the expected behavior? I would've expected that the necessary PVCs be auto-created (and automatically filled as long as a PV is available). Instead, creating the PetSet before the PVC does create a "zombie" PVC which is never filled, regardless of what I do.

Sorry about asking here, but I've looked around and wasn't able to find any resources on this. I'm sure they exist somewhere; appreciate a pointer.

@bprashanth
Copy link

bprashanth commented Jul 3, 2016

@ingvagabund if you actually want to invoke the dynamic provisioner you need to specify https://github.com/kubernetes/kubernetes.github.io/blob/release-1.3/docs/user-guide/petset.yaml#L43, the way you currently have it, the petset is asking for a specific pvc

@tschottdorf see 4 in https://github.com/kubernetes/kubernetes.github.io/blob/release-1.3/docs/user-guide/petset.md#alpha-limitations and https://github.com/kubernetes/kubernetes.github.io/blob/release-1.3/docs/user-guide/petset.md#stable-storage

The provisioners are started based on your environment. From https://github.com/kubernetes/kubernetes/blob/release-1.3/examples/experimental/persistent-volume-provisioning/README.md

No configuration is required by the admin! 3 cloud providers will be provided in the alpha version of this feature: EBS, GCE, and Cinder.

@smarterclayton
Copy link

As noted on the cockroach thread you can do:

kind: PersistentVolume
apiVersion: v1
metadata:
  name: pv-host-1
spec:
  hostPath:
    path: /var/lib/random/dir-1

as necessary (you may have to set capacity and length).

@bprashanth
Copy link

@ingvagabund did #1295 (comment) work?

@ingvagabund
Copy link
Contributor Author

@bprashanth I will check that out. Sorry for the delay.

@ingvagabund
Copy link
Contributor Author

@bprashanth the etcd petset is now scalable. Though, I have hit some issues when scaling up and down:

  • When running kubectl patch petset/etcd -p '{"spec":{"replicas": 3}}' from 5 to 3 replicas, pets etcd-4 and etcd-5 were not deleted.
  • When running kubectl edit and changing number of replicas from 13 to 5, pets etcd-5 and etcd-6 were not deleted. After removing etcd-6, both pets got deleted.

It does not have to be necessary related to the petset controller. Maybe something wrong in the specification.

```

1. run skydns
1. create the petset

## Scaling

TODO
The etcd cluster can be scale up by running ``kubectl patch`` or ``kubectl edit``. For instance,
Copy link
Contributor

Choose a reason for hiding this comment

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

kubectl scale?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$ kubectl scale --replicas=5 petset/etcd
error: no scaler has been implemented for {"apps" "PetSet"}

@ingvagabund
Copy link
Contributor Author

Scaling up from 3 to 13 and back, the middle state of the petset is

NAME                 READY     STATUS    RESTARTS   AGE
etcd-0               1/1       Running   0          3m
etcd-1               1/1       Running   0          3m
etcd-11              1/1       Running   0          3m
etcd-12              1/1       Running   0          3m
etcd-2               1/1       Running   0          3m
etcd-3               1/1       Running   1          3m

Don't think pets etcd-12 and etcd-11 should exist if etcd-10 is gone.

@bprashanth
Copy link

Scaling will stop if a pet is unhealthy. It's possible that there are issues that need to be fixed when scaling halts because of an unhealth pet, and then the user scales up/down. The behavior currently is undefined in such situations. Scaling up should happen in order, once the petset is stable, scaling down should happen in reverse order. kubernetes/kubernetes#30082

done

PEERS=""
for i in $(seq 0 $((${INITIAL_CLUSTER_SIZE} - 1))); do

Choose a reason for hiding this comment

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

this stuff seems like exactly the type of thing to put in the init container (see zookeeper example:), any reason it should be in entrypoint? https://github.com/kubernetes/contrib/tree/master/pets/zookeeper/init. Putting it in init allows one to maintain both images independently, but I'm ok doing it this way if there are some limitations.

Choose a reason for hiding this comment

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

putting it in init also gives you a clear signal that bootstrapping/joining cluster failed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this stuff seems like exactly the type of thing to put in the init container (see zookeeper example:), any reason it should be in entrypoint?

There is nothing to install, setup/configure or bootstrap. Initial cluster bootstrapping consists of waiting for initial pets. The same holds for the adding post-bootstrap members. The script "just" adds new member and start etcd.

Putting it in init allows one to maintain both images independently, but I'm ok doing it this way if there are some limitations.

As all the code is basically written over etcd image, there is no need for additional one.

putting it in init also gives you a clear signal that bootstrapping/joining cluster failed

Would not be postStart better? Basically, the main container command itself is to run etcd. The current code "just" collects required envs, params, puts them together and start etcd. So one either add member, remove member or update member. That is the only piece of logic worth putting into initialization part. Unfortunately, all three etcd commands differs in input options which are populated during initialization. So separation of initialization code and executing etcd itself does not make sense.

Choose a reason for hiding this comment

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

Both are valid cases. I guess the question comes down to consistency

@bprashanth
Copy link

I'm planning on waiting for consensus on https://groups.google.com/forum/#!topic/kubernetes-dev/MhQl2GBylgM (I think we're close) before doing a next pass. Hoping we can directly check this in there. Let me know if you'd rather check it in here and transition instead and I can do a review.

@ingvagabund
Copy link
Contributor Author

ingvagabund commented Aug 11, 2016

I am fine with waiting. At the same time (and as was commented in the discussion) we need some sort of CI to at least test 1) correctness and 2) regressions.

@mdshuai
Copy link
Contributor

mdshuai commented Aug 22, 2016

@ingvagabund Is the etcd pod must run as root and in etcd namespaces?

@ingvagabund
Copy link
Contributor Author

@ingvagabund Is the etcd pod must run as root and in etcd namespaces?

Etcd process runs as a root by default unless specified otherwise. Not aware of any etcd namespace requirement. What is your usecase here to test?

# re-join member
ETCDCTL_ENDPOINT=$(eps) etcdctl member update ${member_id} http://${HOSTNAME}.${PETSET_NAME}:2380
exec etcd --name ${HOSTNAME} \
--listen-peer-urls http://${HOSTNAME}.${PETSET_NAME}:2380 \
Copy link
Contributor

@mdshuai mdshuai Aug 24, 2016

Choose a reason for hiding this comment

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

{HOSTNAME}.${PETSET_NAME} can't be resolve by dns. should be {HOSTNAME}.${SERVICE_NAME}, in this example, you set petse-tname equal with svc-name so it can running well. Need add another env SERVER_NAME to replace some fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you dump entries in your DNS? I have never had this problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

In this design doc https://github.com/bprashanth/kubernetes.github.io/blob/4df7d96369964a267466bdffc73f7d343c77584f/docs/user-guide/petset.md#network-identity As each pet is created, it gets a matching DNS subdomain, taking the form: $(petname).$(governing service domain), where the governing service is defined by the serviceName field on the Pet Set.
If you set petsetname don't equal to svc name, then you can see this issue.

@test-foxish
Copy link

recomputing cla status...

@k8s-github-robot
Copy link

[CLA-PING] @ingvagabund

Thanks for your pull request. It looks like this may be your first contribution to a CNCF open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://identity.linuxfoundation.org/projects/cncf to sign.

Once you've signed, please reply here (e.g. "I signed it!") and we'll verify. Thanks.


@k8s-github-robot k8s-github-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Sep 22, 2016
@k8s-github-robot
Copy link

[CLA-PING] @ingvagabund

Thanks for your pull request. It looks like this may be your first contribution to a CNCF open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://identity.linuxfoundation.org/projects/cncf to sign.

Once you've signed, please reply here (e.g. "I signed it!") and we'll verify. Thanks.


@k8s-github-robot k8s-github-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Oct 2, 2016
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] The Following OWNERS Files Need Approval:

  • pets

We suggest the following people:
cc @bprashanth
You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancelin a comment

@foxish
Copy link
Contributor

foxish commented Aug 9, 2017

Closing this, it should probably exist as an example outside contrib.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/demos area/pets cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.