Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Memcached as a Service Support #15

Closed
wants to merge 10 commits into from
Closed

Memcached as a Service Support #15

wants to merge 10 commits into from

Conversation

oza
Copy link

@oza oza commented Jan 1, 2012

Memcached is a de fact standard in-memory caching system.
This patch provides Cloud Foudnry with the provisioning, unprovisioning Memcached instances.

And, happy new year!

Signed-off-by: OZAWA Tsuyoshi ozawa.tsuyoshi@gmail.comDear Cloud Foundry contributor,

If you are reading this message, it means you submitted a pull request in the Cloud Foundry GitHub repository.

First of all, thanks! We really appreciate your participation.

Recently we made some changes in how we are verifying and reviewing open source contributions like yours. In addition, we changed the way we can expose our internal development in real-time. The changes are exciting, as they allow all our staff to collaborate seamlessly with you, which increases our mutual velocity and gives the community a bigger stake in our direction.

The Cloud Foundry team uses Gerrit, a code review tool that originated in the Android Open Source Project. We also use GitHub as an official mirror, though all pull requests are accepted via Gerrit.

Follow these steps to make a contribution to any of our open source repositories:

  1. Complete our CLA Agreement for
    individuals or
    corporations.

  2. Sign up for an account on our public Gerrit server at
    http://reviews.cloudfoundry.org/.

  3. Create and upload your public SSH key in your Gerrit account profile.

  4. Set your name and email:

            git config --global user.name "Firstname Lastname"
            git config --global user.email "your_email@youremail.com"
    
  5. Install our gerrit-cli gem:

            gem install gerrit-cli
    
  6. Clone the Cloud Foundry repo:
    Note: to clone the BOSH repo, or the Documentation repo, replace
    vcap with bosh or oss-docs

            gerrit clone ssh://reviews.cloudfoundry.org:29418/vcap
            cd vcap
    
  7. Make your changes, commit, and push to gerrit:

            git commit
            gerrit push
    

Once your commits are approved by our Continuous Integration Bot (CI Bot) as well as our engineering staff, return to the Gerrit interface and MERGE your changes. The merge will be replicated to GitHub automatically at https://github.com/cloudfoundry. If you get feedback on your submission, we recommend squashing your commit with the original change-id. See the squashing section here for more details: https://help.github.com/rebase.

# 31100 - 31199 Memcached-specific Error
# FIXME: This error code is paste from redis.
MEMCACHED_SAVE_INSTANCE_FAILED = [31100, HTTP_INTERNAL, "Could not save instance: %s"]
MEMCACHED_DESTORY_INSTANCE_FAILED = [31101, HTTP_INTERNAL, "Could not destroy instance: %s"]

Choose a reason for hiding this comment

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

Typo.. "DESTORY"

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your pointing out. I'll fix it.

@krobertson
Copy link

+1. I was pondering writing one as well.

It looks like there is still some commented out parts referring to redis, and I do agree some authentication would be good with like SASL.

@oza
Copy link
Author

oza commented Jan 3, 2012

@yssk22r @krobertson Thank you for your advice. I'm going to support authentication with SASL and delete needless source code.

UEMURA Yuichi added 2 commits January 10, 2012 02:01
Memcached is a de fact standard in-memory caching system.
This patch provides Cloud Foundry with the provisioning,
unprovisioning Memcached instances.

Some advices are given by:

Ken Robertson <ken _at_ invalidlogic.com>
Yohei Sasaki  <yssk22 _at_ gmail.com>
Ilya Grigorik <ilya _at_ igvita.com>

Signed-off-by: UEMURA Yuichi <y.uemura@ntt.com>
In PaaS environment, the authentication feature is required.
This patch provides MaaS with SASL support of memcached.

Signed-off-by: UEMURA Yuichi <y.uemura@ntt.com>
@natea
Copy link

natea commented Jan 24, 2012

Would be awesome to have memcached-as-a-service in CloudFoundry!

@krobertson
Copy link

Looks good! Definitely going to pull this into my code tree for PaaS.io! Will report back once I test it out.

varz[:plan] = instance.plan
varz[:usage] = {}
varz[:usage][:max_memory] = instance.memory.to_f * 1024.0
# FIXME : get more detail information
Copy link

Choose a reason for hiding this comment

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

Usage info will be necessary to merge this.

Copy link
Author

Choose a reason for hiding this comment

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

@oza
Copy link
Author

oza commented Jan 25, 2012

@krobertson @leto Thank you for your review! I'll fix the parts you pointed out, and commit to the memcached branch.

UEMURA Yuichi added 5 commits January 30, 2012 22:46
Signed-off-by: UEMURA Yuichi <y.uemura@ntt.com>
Signed-off-by: UEMURA Yuichi <y.uemura@ntt.com>
Signed-off-by: UEMURA Yuichi <y.uemura@ntt.com>
There are several fixes including:
	1. avoiding instances leak.
	2. authentication supports.
	3. SASLAdmin's handling empty inputs.

For more detail, Please see the source codes under the spec directory.

Signed-off-by: UEMURA Yuichi <y.uemura@ntt.com>
Signed-off-by: UEMURA Yuichi <y.uemura@ntt.com>
@oza
Copy link
Author

oza commented Feb 1, 2012

Now, all tasks are done. Please review the codes.

@oza
Copy link
Author

oza commented Feb 2, 2012

@leto Thanks :-)

@oza
Copy link
Author

oza commented Feb 3, 2012

@leto Please let me know if there are some points to fix to merge this patch.

@krobertson
Copy link

Sorry, haven't had a chance yet to review it some more but definitely will be.

@leto
Copy link

leto commented Feb 3, 2012

@oza we are transitioning to a new review system, if you are confident with the current patch, I can help push it through :) Have you ever used Gerrit?

@oza
Copy link
Author

oza commented Feb 6, 2012

@krobertson I'm looking forward to your reviewing :-)

@oza
Copy link
Author

oza commented Feb 6, 2012

@leto In fact, I've never used Gerrit. I'm going to learn it, however, if your new review system use it.

@leto
Copy link

leto commented Feb 8, 2012

@oza yes, we will continue to mirror to Github, but very soon we will have a public Gerrit instance so that external and internal developers will be working together and using the same workflow.

@oza
Copy link
Author

oza commented Feb 8, 2012

@leto It's the great news for us! We're looking forward to joining to the exciting collaboration! Please let me know if it becomes available.

@leto
Copy link

leto commented Feb 9, 2012

@oza My highest priority is getting a a public Gerrit instance working. I will definitely let you know when it is close to ready :)

Also, it is our policy to leave copyright lines alone and instead add people/companies who contribute to https://github.com/cloudfoundry/vcap/blob/master/AUTHORS . Could you undo the changes to the copyright lines in this PR and create a new PR for vcap that adds yourself/NTT to AUTHORS?

Once that happens I can push this into our internal Gerrit so we can enjoy memcached support before the public Gerrit is ready :)

Signed-off-by: UEMURA Yuichi <y.uemura@ntt.com>
UEMURA Yuichi added 2 commits February 11, 2012 09:30
This commit includes 2 fixes:
1. Avoid memcached instance leak when running test cases.
2. Fixed some typo.

Signed-off-by: UEMURA Yuichi <y.uemura@ntt.com>
Fixed to call start_instance() in enable_instance().
This change allows MaaS to work rebalancing correctly.

Signed-off-by: UEMURA Yuichi <y.uemura@ntt.com>
true
end

def enable_instance(service_credentials, binding_credentials_map = {})

Choose a reason for hiding this comment

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

I noticed that in disable_instance, there is a call to stop_instance, however in enable_instance, there is no call to start it again.

Copy link
Author

Choose a reason for hiding this comment

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

@krobertson Thank you for your review! This bug may cause unexpected case(especially when rebalancing), therefore I fixed at https://github.com/nttcom/vcap-services/commit/93a6a291881a4caacf5dda941838565286778e40

@oza
Copy link
Author

oza commented Feb 14, 2012

@leto OK.

At first, I fixed the copyrights for merging at https://github.com/nttcom/vcap-services/commit/eff7956c08fff9931fe70230ac77b9b36e3cd960

Also, we'd like to add our name to the AUTHOR file, and should I send pull request to vcap itself instead of vcap-services?

@leto
Copy link

leto commented Feb 14, 2012

@oza, yes since vcap is the main repo and all others are submodules inside of vcap, we just keep one AUTHORS file.

@oza
Copy link
Author

oza commented Feb 17, 2012

@leto We sent the pull request! Please check out: cloudfoundry-attic/vcap#185

@oza
Copy link
Author

oza commented Feb 23, 2012

@leto Please notify me if there appear tasks which I should do or I can help.

@oza
Copy link
Author

oza commented Mar 8, 2012

Wow, big changes came. We'll start to rebase to catch up vmware changes!
Please give me comments if there appear some fix points to merge. > commiters

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.

6 participants