As $subj says please add those variables to a list on input-variables.md page as it's not straightforward one should modify those in case they don't want to use their own user ocid which is currently the default.

Add pid in logs to make it easier to track a request.

Currently, it the system test if it gets cancelled part way through then the cluster can end up in a state where one or the nodes are cordoned off. We need to ensure that either we clean up properly, or when we start a new test, we ensure that the cluster is in a good state. Initially, this might be as simple as the following:

• On startup, check that all nodes are schedulable (i.e. not cordoned), and uncordon any that are not.

If the integration test get cancelled part way though, the OCI resources (i.e. instance/volume) failed to get cleaned up. Not sure what we do about this though...

It would be nice if the Make 'all' target was updated to be:
all: gofmt golint govet test build manifests build-integration-tests
instead of:
all: clean test build manifests build-integration-tests

However, currently golint produces a load of errors, so these will need fixing/ignoring first:

pkg/mount/mount.go:17:1: package comment should be of the form "Package mount ..."
pkg/mount/mount.go:34:2: exported const MountsInGlobalPDPath should have comment (or a comment on this block) or be unexported
pkg/mount/mount.go:37:6: exported type Interface should have comment or be unexported
pkg/mount/mount.go:64:1: comment on exported type MountPoint should be of the form "MountPoint ..." (with optional leading article)
pkg/mount/mount.go:65:6: type name will be used as mount.MountPoint by other packages, and that stutters; consider calling this Point
pkg/mount/mount.go:142:1: comment on exported function GetDeviceNameFromMount should be of the form "GetDeviceNameFromMount ..."
pkg/mount/mount_unsupported.go:21:6: exported type Mounter should have comment or be unexported
pkg/mount/mount_unsupported.go:25:1: exported method Mounter.Mount should have comment or be unexported
pkg/mount/mount_unsupported.go:29:1: exported method Mounter.Unmount should have comment or be unexported
pkg/mount/mount_unsupported.go:33:1: exported method Mounter.List should have comment or be unexported
pkg/mount/mount_unsupported.go:37:1: exported method Mounter.IsLikelyNotMountPoint should have comment or be unexported
pkg/mount/mount_unsupported.go:41:1: exported method Mounter.GetDeviceNameFromMount should have comment or be unexported
pkg/mount/mount_unsupported.go:45:1: exported method Mounter.DeviceOpened should have comment or be unexported
pkg/mount/mount_unsupported.go:49:1: exported method Mounter.PathIsDevice should have comment or be unexported
pkg/mount/mount_unsupported.go:61:1: exported function IsNotMountPoint should have comment or be unexported
pkg/oci/client/oci.go:47:23: interface method parameter volumeId should be volumeID
pkg/oci/client/oci.go:51:24: interface method parameter volumeAttachmentId should be volumeAttachmentID
pkg/oci/client/oci.go:59:15: interface method parameter instanceId should be instanceID
pkg/oci/client/oci.go:59:27: interface method parameter volumeId should be volumeID
pkg/oci/client/oci.go:63:15: interface method parameter volumeAttachmentId should be volumeAttachmentID
pkg/oci/client/oci.go:67:24: interface method parameter volumeAttachmentId should be volumeAttachmentID
pkg/oci/client/oci.go:124:40: method parameter volumeAttachmentId should be volumeAttachmentID
pkg/oci/client/oci.go:154:39: method parameter volumeId should be volumeID
pkg/oci/client/oci.go:386:31: method parameter instanceId should be instanceID
pkg/oci/client/oci.go:386:43: method parameter volumeId should be volumeID
pkg/oci/client/oci.go:405:31: method parameter volumeAttachmentId should be volumeAttachmentID
pkg/oci/client/oci.go:423:40: method parameter volumeAttachmentId should be volumeAttachmentID
pkg/oci/client/cache/ocicache.go:26:6: exported type OCICache should have comment or be unexported

Original comment

Prior to merging it would be good to get an integration test in covering using full OCIDs and an update to the documentation to differentiate between the restrictions when using spec.volumes[].flexvolume and spec.volumes[].persistentVolumeClaim.

The driver uses the region key (e.g phx) when dynamically creating OCIDs. Unfortunately there are inconsistencies across regions. For example

A volume in PHX uses the region key

“ocid1.volume.oc1.phx.abyhqljrsvvjw3qvfg52n3dmhh7yt3bo6zuprwmvefwx3n5xyhsiyprs6gpa”

But in frankfurt it doesn’t use the key it uses the region itself

“ocid1.volume.oc1.eu-frankfurt-1.abtheljsch5y7wfnk5jyadulsjnbpl5t7afrpbneh5rdh4gg22hlf4u3asra”

Support instance principals as an alternative mechanism of authentication.

See oracle/terraform-kubernetes-installer#174

The test image we deliver is useful to users of the volume provisioner, i.e. you can run it in a cluster to check that the deployed provisioner is operational. We need to document how to do this.

It is possible when doing a release that we overwrite an existing release by re-pushing an image.

To avoid this the release pipeline should fail if a git tag already exists.

if git rev-parse "$VERSION" >/dev/null 2>&1; then	
    echo "Tag $VERSION already exists. Doing nothing."	
    exit 1	
fi

Another option

- script:
  name: Ensure version is unqiue
  code: |
    if curl -s https://api.github.com/repos/oracle/oci-cloud-controller-manager/git/refs/tags | grep "tags/$VERSION"; then exit 1; fi

Currently integration tests are run in the bristol-cloud compartment. They should be run in the kubernetes-test compartment.

Currently the system tests install the driver by shelling out to scp from Python. The documented installation method, however, uses the provided Ansible role. For the system tests to be a truely end-to-end test we should install the driver using the Ansible role.

We should deploy the oci flexvolume driver as a daemon set so that users don't have to manually copy the flex binary on to each node

Check the status of daemonset.
Print out version of drvier in deploy.sh and check this via kubectl logs

Currently the configuration has to live in the same directory as the binary, which means we can't mount in a kubernetes secret to the same directory.

This will enable self-hosted deployments to use a side-car + secret with the kube controller manager.

The daemonset should require a secret has been created.

This should be like the volume config (Or maybe the same secret) and mounted just on the master.

This will require creating two daemonsets, one which runs on the master (and copies the config secret to distk) and one that runs on the nodes (that doesn't copy the secret)

Docs have.

NOTE: If running kube-controller-managers in a container you must ensure that the plugin directory is mounted into the container. See: https://gitlab-odx.oracle.com/odx/oke-prime/merge_requests/15 for specifics.

It should reference the kubeadm issue

The flexdriver currently logs to a file which makes debugging in certain environments difficult.

We should expose these logs via kubectl so that customers can access logs without needing access to physical machines.

See https://kubernetes.io/docs/concepts/cluster-administration/logging/#streaming-sidecar-container for an example of how to do this with a sidecar container

Is it still necessary for the region_key to be required? It should be removed if this is the case.

Prior to merging #11 SCP/SSHing to the master node in the system tests to run the test made sense as we were provisioning the cluster in that fashion.

Now that we are provisioning the cluster with Ansible the tests could be considerably simplified by passing $KUBECONFIG in the Wercker environment and running the kubectl commands from the pipeline rather than on the master node.

Currently both the system and the integration tests use an API signing key that is not associated with an account scoped to the kubernetes-test compartment (AFAIK).

We should also rotate the instance (ssh) key as well given that they are/have been associated with instances deployed into the bristol-cloud compartment.

This is a precautionary step. There is no reason to suspect we have leaked any credentials but taking this precaution prior to OSS release seems expedient.

This should be done after #14 has been implemented.

We should skip validation of AuthConfig as the credentials do not get overridden when instance principals is set to true and currently we hard fail which seems unnecessary in this scenario. This could be replaced by simply logging a message to let the user aware that AuthConfig credentials are redundant when IP is used.

The documentation states that the DaemonSet should be installed with:

kubectl apply -f https://github.com/oracle/oci-flexvolume-driver/releases/download/${flexvolume_driver_version}/oci-flexvolume-driver.yaml

However the oci-flexvolume-driver.yaml file doesn't exist in any release. E.g.:

error: unable to read URL "https://github.com/oracle/oci-flexvolume-driver/releases/download/0.6.2/oci-flexvolume-driver.yaml", server reported 404 Not Found, status code=404

Node name to instance OCID resolution is currently implemented by:

• First checking whether or not the given node name matches the display name of a running instance (1 API call)
• If it does not call ListVnicAttachments and page through results (1 API request per page)
• For each VnicAttachment in the ATTACHED state we GET the associated Vnic (1 API request per ATTACHED VnicAttachment in compartment)
• Check node name against the VNICshostnameLabel and publicIP

The main fault of this algorithm is that the number of HTTP requests increases linearly with the number of instances in the compartment (which is strictly equal to or greater than the number of nodes in the cluster). This (understandably) in turn triggers rate limiting when the number of instances is great enough.

Notes:

• A similar scalability problem was resolved in the CCM by using a NodeLister to cache the Nodes in the cluster and then using the Node providerID field.
• The calls that utilise this node name lookup are Attach, Detach, and IsAttached.
• These calls are made from the kube-controller-manager.
• We don't cache GetVnic() calls so every time we hit the rate limit we start over in an infinite loop of rate limiting.

Terraform for OCI recently moved to OEL 7.4. Ansible contains superfluous information about Ubuntu (python3 etc)

Today

However, customers can put resources in arbitrary compartments, and all LIST operations require a compartment. Thus it makes it difficult to know how many compartments may be in scope, in practice customers are split across at least two compartments one for compute resources and one for networking resources.

Instead of using the convention of IP to lookup instances, we should use node.spec.providerId as storage operations should only require the instance id and volume id to create appropriate the attachment.

RE: #18

When a new tag is created in Github, the wercker build should build and push a binary release to GH Releases.

1. Separate wercker pipeline to manually create the tag and upload to github releases
2. Wercker build runs on every tag

When the daemonset is deleted it should clean up anything it put down on the filesystem

At the moment it should just delete the binary but once #109 is implemented it should also delete any credentials that have been places on the disk

Hi,

Do we have anything similar for OCI-Classic on the roadmap ? or do you guys suggest to use something else when running k8s environment on OCI-Classic for PersistentVolumes ?

Any help will be much appreciated.

In order to unify the authentication formats across OCI K8s projects, we should change the auth format in the driver to be consistent with the CCM and volume provisioner.

Example configuration

auth:
  tenancy: ocid1.tenancy.oc1..aaaaaaaatyn7scrtwtq
  user: ocid1.user.oc1..aaaaaaaao235lbcxvdrrqlrp
  key: |
    -----BEGIN RSA PRIVATE KEY-----
    <snip>
    -----END RSA PRIVATE KEY-----
  fingerprint: 4d:f5:ff:0e:a9:10:e8:5a:d3:52:6a:f8:1e:99:a3:47
  region: us-phoenix-1

Currently the Ansible role installs the driver without allowing for configuration of compartment or region (requiring use of the OCI metadata endpoint). The Ansible role should accept compartment and region as arguments and if set should output them in the templated driver config file.

The flex volume driver has the luxury of running on every instance in a cluster, which means it'll have access to the local metadata server. Using the instance id means we can save the unnecessary calls by node name (which could be wrong if using the FQDN of the instance) or the public ip/name approach used by OKE.

We should add support for using this approach instead.

I'm guessing that this just kills the docker container thus the atexit hook in the system test doesnt get called for the cleanup.

The only way I can think of sorting this out is in the case where a lockfile exists, the test will need to check if there is a system test pipeline actually running, and if not, clean the file up before starting.

Currently, this driver uses attach/detach; however, that's not compatible with using kubernetes secrets to pass in the required secrets to the flexvolume driver.

As you can read here, the driver needs to use mount instead.

This would allow users to do

kind: StorageClass
apiVersion: storage.k8s.io/v1beta1
metadata:
  name: oci
provisioner: oracle.com/oci
parameters:
  secretName: flex-volume-driver
  secretNamespace: kube-system

Combined with daemonset deployments you wouldn't need to run anything on the Kuberenetes hosts which is most ideal.

Move our published Docker image builds to ocir.io

Currently OCI API credentials must be present on both the control plane and all nodes in the cluster. Ideally they would only be required in the control plane.

• Update OCIFlexvolumeDriver.Init() to only require credentials be present on the control plane.
• Update OCIFlexvolumeDriver.IsAttached() to determine whether it is being called from the Controller manager or the Kubelet and only use the OCI API in the first case.
• Update OCIFlexvolumeDriver.Attach() to block waiting for the VolumeAttachment to reach the ATTACHED state and pass the templated device path.
• Update OCIFlexvolumeDriver.WaitForAttach() to query the device path rather than the OCI API.

To coincide with the volume provisioner issue raised. Some of the credentials do not make sense under AuthConfig and should perhaps be moved to the higher level Config.

We validate the test image as part of our build pipeline. However, currently we have a very permissive set of RBAC rules for running the test inside of the cluster (see: test/system/run-test-image.yaml.template). Would be nice to tighten these up so they only allow the operations that the test requires.

The configuration is setting defaults for the api region which instance principals require not be set in the configuration validation code. This causes the flex volume driver to not work.

[oci-flex-volume-driver-bhwcf] 3820 2018/07/09 17:14:02 Command result: {"status":"Failure","message":"auth.region: Forbidden: cannot be used when useInstancePrincipals is enabled"}

See oracle/oci-cloud-controller-manager#142

We should document a minimal set of OCI policies needed when creating the driver auth creds

• Consolidate docs/ into README.md
• Document install via downloading latest release
• Developer documentation?

See https://github.com/oracle/oci-flexvolume-driver/blob/master/pkg/oci/client/config.go#L31

var ociRegions = map[string]string{
	"phx": "us-phoenix-1",
	"iad": "us-ashburn-1",
	"fra": "eu-frankfurt-1",
}