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

Refer feedback from #673 and augment documentation #684

Conversation

martindekov
Copy link
Contributor

Refering feedback from the review #673 which adds multiple
namespaces functionality which includes the following:

  • Extend operator readme to mention how to enable the feature
  • Remove mentioning that the operator does not support the feature
  • Replacing TODO context with the context from the request
  • Deferring the closing of the body of the namespaces handler
  • Merging the cluster role configuration so it is used in faas-netes
  • Reusing the logic of namespaces and secrets handling present in
    faas-netes in the operator. Removing the custom implementations
    as they are no longer used

Signed-off-by: Martin Dekov [email protected]

Description

Motivation and Context

How Has This Been Tested?

Installed local chart for faas-netes and operator and built custom image with the changes stored and pushed in martindekov/faas-netes:asd-3:

helm upgrade --install openfaas ./openfaas --namespace openfaas --values ./openfaas/values.yaml --set operator.create=<true/false> --set openfaasImagePullPolicy=Always --set operator.image=martindekov/faas-netes:asd-3 --set clusterRole=true --set gateway.directFunctions=true --set faasnetes.imagePullPolicy=Always --set basic_auth=false

Tested the operator to check whether the logic for namespaces and secrets is reused properly. Also deployed a function and invoked it in default and custom namespace both in the operator and faas-netes. Also tested whether faas-netes works with the cluster role as expected.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Feedback from review Add multiple namespaces in the operator #673

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Refering feedback from the review openfaas#673 which adds multiple
namespaces functionality which includes the following:
* Extend operator readme to mention how to enable the feature
* Remove mentioning that the operator does not support the feature
* Replacing TODO context with the context from the request
* Deferring the closing of the body of the namespaces handler
* Merging the cluster role configuration so it is used in faas-netes
* Reusing the logic of namespaces and secrets handling present in
  faas-netes in the operator. Removing the custom implementations
  as they are no longer used

Signed-off-by: Martin Dekov <[email protected]>
Copy link
Member

@alexellis alexellis left a comment

Choose a reason for hiding this comment

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

Approved

@alexellis alexellis merged commit 54ffc9f into openfaas:master Sep 17, 2020
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.

2 participants