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

support Azure File Service volume #17221

Merged
merged 1 commit into from Feb 9, 2016

Conversation

Projects
None yet
@rootfs
Copy link
Member

rootfs commented Nov 13, 2015

Azure File Storage provides a SMB 3.0 file share.

This implementation enables containers running on Azure to access file share located in the same Azure region.

@k8s-merge-robot

This comment has been minimized.

Copy link
Contributor

k8s-merge-robot commented Nov 13, 2015

Labelling this PR as size/XL

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Nov 13, 2015

GCE e2e test build/test passed for commit d2323a5.

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Nov 13, 2015

GCE e2e test build/test passed for commit 4914da0.

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Nov 16, 2015

GCE e2e build/test failed for commit bf6c7e5.

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Nov 16, 2015

GCE e2e build/test failed for commit b943fd7.

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Nov 16, 2015

GCE e2e build/test failed for commit b3957ce.

- name: azure
azureFile:
accountName: account_name
accountKey: account_key

This comment has been minimized.

@swagiaal

swagiaal Nov 16, 2015

Contributor

Hmm.. other implementations gce_pd and aws_ebs for example have kept account information outside the volume definition. Is it possible to do that with Azure ? Perhaps there is a way to authorize the instance through the Azure. I think that would be a better long term strategy especially after the implementation of the Attach/Detach controller.

This comment has been minimized.

@markturansky

markturansky Nov 23, 2015

Member

I agree with @swagiaal. We should not embed the account info directly into the volume.

Why not a CloudProvider like aws/gce/cinder?

This comment has been minimized.

@rootfs

rootfs Nov 24, 2015

Member

this is updated.

This comment has been minimized.

@rootfs

rootfs Nov 24, 2015

Member

azure has separate storage and compute accounts. In AWS and GCE we use instance because we provision block storage to instance. Azure File storage can be provisioned without an instance. You can even access file share off Azure cloud.

@@ -617,6 +621,19 @@ type DownwardAPIVolumeFile struct {
FieldRef ObjectFieldSelector `json:"fieldRef"`
}

// AzureFile represents an Azure File Service mount on the host that shares a pod's lifetime

This comment has been minimized.

@swagiaal

swagiaal Nov 16, 2015

Contributor

s/AzureFile/AzureFileVolumeSource

This comment has been minimized.

@rootfs

rootfs Nov 17, 2015

Member

AzureFile is the variable name, the type is AzureFileVolumeSource

@@ -737,6 +741,19 @@ type FCVolumeSource struct {
ReadOnly bool `json:"readOnly,omitempty"`
}

// AzureFile represents an Azure File Service mount on the host that shares a pod's lifetime

This comment has been minimized.

@swagiaal

swagiaal Nov 16, 2015

Contributor

s/AzureFile/AzureFileVolumeSource

}

func (plugin *azureFilePlugin) CanSupport(spec *volume.Spec) bool {
//TODO: check if mount.cifs is there

This comment has been minimized.

@swagiaal

swagiaal Nov 16, 2015

Contributor

You should be able to use this #16950 soon

os.MkdirAll(dir, 0750)
source := fmt.Sprintf("//%s.file.core.windows.net/%s", b.accountName, b.shareName)
// parameters suggested by https://azure.microsoft.com/en-us/documentation/articles/storage-how-to-use-files-linux/
options := []string{fmt.Sprintf("vers=3.0,username=%s,password=%s,dir_mode=0777,file_mode=0777", b.accountName, b.accountKey)}

This comment has been minimized.

@swagiaal

swagiaal Nov 16, 2015

Contributor

Hmm if accountKey is your password then revealing it in the pod definition might be problematic.

This comment has been minimized.

@rootfs

rootfs Nov 16, 2015

Member

not any more
@swagiaal what happened to codegen? it generated lots of noise

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Nov 16, 2015

GCE e2e build/test failed for commit b61eb5a.

@rootfs rootfs force-pushed the rootfs:azure branch 2 times, most recently from f5883c2 to d263aa2 Nov 16, 2015

@k8s-merge-robot

This comment has been minimized.

Copy link
Contributor

k8s-merge-robot commented Nov 16, 2015

Labelling this PR as size/XXL

@k8s-merge-robot k8s-merge-robot added size/XXL and removed size/XL labels Nov 16, 2015

KeyName string `json:"keyName"`
// Required: Share Name
ShareName string `json:"shareName"`
// Optional: Defaults to false (read/write). ReadOnly here will force

This comment has been minimized.

@pmorie

pmorie Nov 16, 2015

Member

I have received guidance from @bgrant0607 to avoid spelling out optional or required in comments for fields and use omitempty exclusively to indicate when something is optional.

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Nov 16, 2015

GCE e2e build/test failed for commit f5883c2.

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Nov 16, 2015

GCE e2e build/test failed for commit 4f64192.

@rootfs rootfs force-pushed the rootfs:azure branch 2 times, most recently from e01f91b to 0762f58 Nov 17, 2015

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Nov 17, 2015

GCE e2e build/test failed for commit e01f91b.

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Nov 17, 2015

GCE e2e test build/test passed for commit 5201d9b.

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Nov 17, 2015

GCE e2e build/test failed for commit 0762f58.

@rootfs rootfs force-pushed the rootfs:azure branch from 0762f58 to b528afc Nov 17, 2015

@rootfs

This comment has been minimized.

Copy link
Member

rootfs commented Feb 8, 2016

@saad-ali Updated. PTAL. thanks

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Feb 8, 2016

GCE e2e build/test failed for commit 7ea0a1d.

@rootfs rootfs force-pushed the rootfs:azure branch from 36363ea to 9abdc17 Feb 8, 2016

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Feb 8, 2016

GCE e2e build/test failed for commit 36363ea.

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Feb 8, 2016

GCE e2e test build/test passed for commit 9abdc17.

@rootfs rootfs force-pushed the rootfs:azure branch from 9abdc17 to 30e0548 Feb 8, 2016

@k8s-merge-robot

This comment has been minimized.

Copy link
Contributor

k8s-merge-robot commented Feb 8, 2016

Labelling this PR as size/XL

@k8s-merge-robot k8s-merge-robot added size/XL and removed size/XS labels Feb 8, 2016

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Feb 8, 2016

GCE e2e test build/test passed for commit 30e0548.

@k8s-merge-robot

This comment has been minimized.

Copy link
Contributor

k8s-merge-robot commented Feb 8, 2016

PR needs rebase


type azureSvc struct{}

func (s *azureSvc) SetupAzureFileSvc(host volume.VolumeHost, nameSpace, secretName, shareName string) (string, string, error) {

This comment has been minimized.

@saad-ali

saad-ali Feb 8, 2016

Member

nit: Method name no longer accurate. Maybe GetAzureCredentials?

@saad-ali

This comment has been minimized.

Copy link
Member

saad-ali commented Feb 8, 2016

LGTM mod nit + rebase.

support Azure File Service volume
Signed-off-by: Huamin Chen <hchen@redhat.com>

@rootfs rootfs force-pushed the rootfs:azure branch from 30e0548 to d7e4b82 Feb 9, 2016

@rootfs

This comment has been minimized.

Copy link
Member

rootfs commented Feb 9, 2016

@saad-ali updated and rebased. thanks.

@saad-ali

This comment has been minimized.

Copy link
Member

saad-ali commented Feb 9, 2016

LGTM

Thanks @rootfs

@saad-ali saad-ali added the lgtm label Feb 9, 2016

@k8s-teamcity-mesosphere

This comment has been minimized.

Copy link

k8s-teamcity-mesosphere commented on d7e4b82 Feb 9, 2016

TeamCity OSS :: Kubernetes Mesos :: 4 - Smoke Tests Build 14987 outcome was SUCCESS
Summary: Tests passed: 1, ignored: 225 Build time: 00:04:03

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Feb 9, 2016

GCE e2e test build/test passed for commit d7e4b82.

@k8s-merge-robot

This comment has been minimized.

Copy link
Contributor

k8s-merge-robot commented Feb 9, 2016

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Feb 9, 2016

GCE e2e test build/test passed for commit d7e4b82.

@k8s-merge-robot

This comment has been minimized.

Copy link
Contributor

k8s-merge-robot commented Feb 9, 2016

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot

This comment has been minimized.

Copy link

k8s-bot commented Feb 9, 2016

GCE e2e test build/test passed for commit d7e4b82.

@k8s-merge-robot

This comment has been minimized.

Copy link
Contributor

k8s-merge-robot commented Feb 9, 2016

Automatic merge from submit-queue

k8s-merge-robot added a commit that referenced this pull request Feb 9, 2016

Merge pull request #17221 from rootfs/azure
Auto commit by PR queue bot

@k8s-merge-robot k8s-merge-robot merged commit beb5d01 into kubernetes:master Feb 9, 2016

3 of 4 checks passed

Submit Queue Github CI tests are not green.
Details
Jenkins GCE e2e 226 tests run, 106 skipped, 0 failed.
Details
Jenkins unit/integration 2724 tests run, 9 skipped, 0 failed.
Details
cla/google All necessary CLAs are signed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment