Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

DS8K support : ubiquity-db pv name as configurable parameter for the installer. #167

Merged
merged 4 commits into from
Mar 1, 2018

Conversation

hfeish
Copy link
Contributor

@hfeish hfeish commented Feb 7, 2018

Change:
In ubiquity 1.0.0, ibm-ubiquity-db is a hardcord pv name, but to support DS8k which has volume name length limitation about 16 chars, so we change design about ubiquity db name, make it configurable in ubiquity_installer.conf and ubiquity-configmap.yml, so customer can set the default name "ibm-ubiquity-db" to example "ibmdb" if the backend storage is DS8k

How to test:
basic testing:

  1. The default db name works on SVC/XIV, and not works on DS8k
  2. Change default db name to "ibmdb" and make sure it works on DS8k
  3. Change db name to "ibm-ubiquity-db-test", and it should also works on SVC/XIV
  4. Verify uninstall successfully

upgrade testing:
steps:
Had ubiquity 1.0.0 installed, backend storage is SVC/XIV
Upgrade ubiquity-configmap.yml with ubiquity_installer.conf(use default db name)
Upgrade configmap with new ubiquity-configmap yml
Upgrade Ubiquity and Provisioner Pod
Make sure everything works fine

Documents:
1. ibm-ubiquity-db is the default db name for SVC/XIV, for DS8k, we suggest to use ibmdb, also customer should know that db name: u_{instance}_ibmdb should less than 17 chars
2. if customer want to create pv in DS8k, customer need to set pv name with pv-name label in pvc, otherwize it will fail to create pvc because DS8k has 16 chars limitation
3. Need to tell customer how to do upgrade

Signed-off-by: feihuang feihuang@feihuangs-mbp.cn.ibm.com


This change is Reviewable

Signed-off-by: feihuang <feihuang@feihuangs-mbp.cn.ibm.com>
@hfeish hfeish requested review from shay-berman and alonm February 7, 2018 06:19
@hfeish hfeish changed the title update ibm-ubiquity-db to configurable change ibm-ubiquity-db to configurable Feb 7, 2018
@shay-berman
Copy link
Contributor

Review status: 0 of 6 files reviewed at latest revision, 6 unresolved discussions.


scripts/installer-for-ibm-storage-enabler-for-containers/ubiquity-configmap.yml, line 29 at r1 (raw file):

   DEFAULT-FSTYPE: DEFAULT_FSTYPE_VALUE

   # Ubiquity db pv name, For SVC/XIV, use default value. For DS8k, please set a short name, ex: ibmdb, because DS8k has a limitation for 16 chars volume name

don't instruct here to set the param, because this param cannot be changed after the first installation.
The explanation should be in the ubiquiyt_installer.conf.


scripts/installer-for-ibm-storage-enabler-for-containers/ubiquity_cli.sh, line 163 at r1 (raw file):

    echo ""

    pvname=`kubectl get $nsf pvc ${UBIQUITY_DB_PVC_NAME} --no-headers -o custom-columns=name:spec.volumeName`

what about the pvc name? don't we align it with the PV name?


scripts/installer-for-ibm-storage-enabler-for-containers/ubiquity_installer.conf, line 40 at r1 (raw file):


# Parameters in ubiquity-configmap.yml that impact on ubiquity db volume
#-----------------------------------------------------------------

Param...

#--------------------
not sure you need this comments


scripts/installer-for-ibm-storage-enabler-for-containers/ubiquity_installer.sh, line 198 at r1 (raw file):

    KEY_FILE_DICT['UBIQUITY_K8S_FLEX_IMAGE']="${UBIQUITY_FLEX_DAEMONSET_YML}"
    KEY_FILE_DICT['SCBE_MANAGEMENT_IP_VALUE']="${UBIQUITY_CONFIGMAP_YML}"
    KEY_FILE_DICT['SCBE_MANAGEMENT_PORT_VALUE']="${UBIQUITY_CONFIGMAP_YML}"

r u sure nothing need to be change in the uninstall? probably no, but please check


scripts/installer-for-ibm-storage-enabler-for-containers/yamls/ubiquity-db-pvc.yml, line 4 at r1 (raw file):

apiVersion: v1
metadata:
  name: "ibm-ubiquity-db"

I am not sure, maybe its better to align also the pvc name according to the pv?
not sure, please consider the implication if any.


scripts/installer-for-ibm-storage-enabler-for-containers/yamls/ubiquity-db-pvc.yml, line 8 at r1 (raw file):

    volume.beta.kubernetes.io/storage-class: "STORAGE_CLASS_NAME_VALUE"
  labels:
    pv-name: "IBM_UBIQUITY_DB_PV_NAME_VALUE"   # Ubiquity provisioner will create a PV with ibm-ubiquity-db instead of PVC-ID by default.

change the comment : "ibm-ubiquity-db" ->


Comments from Reviewable

@shay-berman shay-berman changed the title change ibm-ubiquity-db to configurable ubiquity-db pv name as configurable parameter for the installer (to support DS8K) Feb 7, 2018
@shay-berman
Copy link
Contributor

FYI : I also update a bit the PR name and description to clarify the change.

@shay-berman
Copy link
Contributor

Reviewed 6 of 6 files at r1.
Review status: all files reviewed at latest revision, 6 unresolved discussions.


Comments from Reviewable

@hfeish
Copy link
Contributor Author

hfeish commented Feb 8, 2018

@shay-berman , Do we need to change pvc name? I think pvc name is a claim for PV, so why not we keep the consistent name for SVC/XIV/DS8k, and also make ibm-ubiquity-db as a specific name for ibm ubiquity db claim.

@shay-berman
Copy link
Contributor

Review status: 3 of 7 files reviewed at latest revision, 7 unresolved discussions.


scripts/installer-for-ibm-storage-enabler-for-containers/ubiquity_cli.sh, line 163 at r1 (raw file):

Previously, shay-berman wrote…

what about the pvc name? don't we align it with the PV name?

ok so we can keep the PVC with the original name and only change the PV name if needed.


scripts/installer-for-ibm-storage-enabler-for-containers/ubiquity_installer.conf, line 39 at r3 (raw file):

DEFAULT_VOLUME_SIZE_VALUE=1

# Ubiquity db pv name, For SVC/XIV, use default value. For DS8k, please set a short name, ex: ibmdb, because DS8k has a limitation for 16 chars volume name

ex -> e.g:


scripts/installer-for-ibm-storage-enabler-for-containers/ubiquity_installer.sh, line 198 at r1 (raw file):

Previously, shay-berman wrote…

r u sure nothing need to be change in the uninstall? probably no, but please check

update - since we only change the PV name, no need to update the uninstall.


scripts/installer-for-ibm-storage-enabler-for-containers/yamls/ubiquity-db-pvc.yml, line 4 at r1 (raw file):

Previously, shay-berman wrote…

I am not sure, maybe its better to align also the pvc name according to the pv?
not sure, please consider the implication if any.

Fie, make sure uninstall align with the new PV configuration (it should be because we just delete the PVC but please double check it)


scripts/installer-for-ibm-storage-enabler-for-containers/yamls/ubiquity-db-pvc.yml, line 8 at r1 (raw file):

Previously, shay-berman wrote…

change the comment : "ibm-ubiquity-db" ->

the comment should be : Ubiquity provisioner will create a PV with dedicated name (by default its ibm-ubiquity-db)


Comments from Reviewable

@hfeish
Copy link
Contributor Author

hfeish commented Feb 28, 2018

@hfeish
Copy link
Contributor Author

hfeish commented Mar 1, 2018

Review status: 3 of 7 files reviewed at latest revision, 7 unresolved discussions.


scripts/installer-for-ibm-storage-enabler-for-containers/ubiquity_installer.conf, line 39 at r3 (raw file):

Previously, shay-berman wrote…

ex -> e.g:

changed


scripts/installer-for-ibm-storage-enabler-for-containers/yamls/ubiquity-db-pvc.yml, line 4 at r1 (raw file):

Previously, shay-berman wrote…

Fie, make sure uninstall align with the new PV configuration (it should be because we just delete the PVC but please double check it)

Yudai tested uninstall in DS8k, and zhiyuan tested uninstall after upgrade from 1.0.0 to 1.1.0, both successful


scripts/installer-for-ibm-storage-enabler-for-containers/yamls/ubiquity-db-pvc.yml, line 8 at r1 (raw file):

Previously, shay-berman wrote…

the comment should be : Ubiquity provisioner will create a PV with dedicated name (by default its ibm-ubiquity-db)

Done


Comments from Reviewable

@shay-berman
Copy link
Contributor

:lgtm:

Thanks @FEI
Please merge to dev


Review status: 3 of 7 files reviewed at latest revision, 4 unresolved discussions.


Comments from Reviewable

@hfeish hfeish merged commit 59ff017 into dev Mar 1, 2018
@hfeish
Copy link
Contributor Author

hfeish commented Mar 1, 2018

Merge and close

@shay-berman shay-berman added this to the DS8K support milestone Mar 11, 2018
@shay-berman shay-berman changed the title ubiquity-db pv name as configurable parameter for the installer (to support DS8K) DS8K support : ubiquity-db pv name as configurable parameter for the installer. Mar 11, 2018
@shay-berman shay-berman deleted the fix/UB-945_Ubiquity_For_DS8k_P3 branch September 17, 2018 09:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants