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.

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

add master-data pvc for etcd #65

Merged
merged 4 commits into from
Jul 17, 2018
Merged

Conversation

mjudeikis
Copy link
Contributor

Add pvc for master-data /etcd container

We could do without storageclass name but left it as we still dont know how final HCP will look like.

@mjudeikis
Copy link
Contributor Author

cc: @pweil- @Kargakis @jim-minter

@0xmichalis
Copy link
Contributor

0xmichalis commented Jul 9, 2018 via email

@mjudeikis
Copy link
Contributor Author

@Kargakis for etcd itself?

@mjudeikis
Copy link
Contributor Author

POC for etcd operator, so we might, can this and move straight to operators. Need to discuss.
#67

@0xmichalis
Copy link
Contributor

Close in favor of #67?

@pweil-
Copy link
Contributor

pweil- commented Jul 12, 2018

we need to be testing real upgrades, this would let us do that while we work on the operator implementation. I'm in favor of closing if we can get the operator in shape quickly (like this week) but I don't want to hold up upgrade testing all next week if it's going to take some time.

@mjudeikis wdyt about the ETA for remaining work on #67?

@mjudeikis
Copy link
Contributor Author

I would say merge and operator will be built ontop? This will enable people to do testing with what we have now. I think a working example for #67 is possible for tomorrow evening.

@0xmichalis
Copy link
Contributor

If we can't have the operator in time then I'd still vote for switching etcd to a statefulset.

@@ -72,6 +72,7 @@ func Generate(cs *acsapi.ContainerService, c *Config) (err error) {
c.Version = versionLatest
c.ImageConfigFormat = "openshift/origin-${component}:${version}"
c.TunnelHostname = strings.Replace(cs.Properties.OrchestratorProfile.OpenShiftConfig.PublicHostname, "openshift", "openshift-tunnel", 1)
c.EtcdPVCStorageClassName = "managed-premium"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not set this directly in the PVC? It's always going to be managed-premium or whatever gives etcd an SSD.

@pweil-
Copy link
Contributor

pweil- commented Jul 12, 2018

I think a working example for #67 is possible for tomorrow evening.

Probably worth holding out then.

@mjudeikis
Copy link
Contributor Author

@pweil- @Kargakis considering my email on etcd-operator and #75 I would merge this.

@jim-minter
Copy link
Contributor

@Kargakis statefulset would be nice but when I tried to implement it I got stuck with kubernetes/kubernetes#52653 - kubectl rollout status statefulset hangs forever if a rollout is not actually happening

@jim-minter
Copy link
Contributor

@mjudeikis pushed some commits to clean up; merging

@jim-minter jim-minter merged commit b3dde3a into openshift:master Jul 17, 2018
@0xmichalis
Copy link
Contributor

kubernetes/kubernetes#62943 needs to be cherrypicked in origin#master and origin#release-3.10

@0xmichalis
Copy link
Contributor

kubernetes/kubernetes#62943 needs to be cherrypicked in origin#master and origin#release-3.10

Hrm, I just realized cherrypicking this back to origin does not help in our case since the issue is still present in the underlying aks cluster :(

mjudeikis added a commit to mjudeikis/openshift-azure that referenced this pull request Jul 18, 2018
@mjudeikis mjudeikis deleted the etcd-pvc branch April 26, 2019 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants