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

Fixing issues with GCR #34

Merged
merged 2 commits into from
Feb 24, 2017
Merged

Conversation

aaron-prindle
Copy link
Contributor

The replicationController.yaml was not properly updated in my original PR. I have added the necessary changes to make the secret mount properly with GCR. For now both secrets need to be added for using either AWS or GCR, we can try to support these being optional with:
https://kubernetes.io/docs/user-guide/secrets/#optional-secrets-from-environment-variables
but this is relatively new so the workaround is that both secrets are required. Currently if either the registry-creds or gcr-creds secret is missing, the pod will not initialize. I have changed the secret.yaml template file to have dummy data for both.

@tsloughter
Copy link

I'm hitting this right now. Is there no way currently to use this addon without having AWS creds... I don't have any and am trying to use minikube with gcr.

@tsloughter
Copy link

This seems to not try GCR after failing the authentication with AWS creds that are invalid.

@aaron-prindle aaron-prindle changed the title Fixed issues with GCR WIP Fixing issues with GCR Feb 11, 2017
@@ -307,7 +307,7 @@ func validateParams() providerConfig {
var ecrEnabled bool

awsAccountID = os.Getenv("awsaccount")
if len(awsAccountID) == 0 {
if len(awsAccountID) == 0 || awsAccountID == "changeme" {
Copy link
Member

Choose a reason for hiding this comment

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

Do you think we should merge the secrets? Then make it more explicit which cloud is desired? I want to get everyone working, but may refactor to have it try and work with any cloud at the same time (or both).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think that merging the secrets is the best approach as all secrets are required for the pod to load. I am debating what is the best way to configure the "mode" though. A config map perhaps?

Copy link
Member

Choose a reason for hiding this comment

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

Oh you did merge to the same! I missed that! I wonder if there's a simple way to "walk" the user through setting it up. Like when you enable the add-on from minikube, it would prompt for the right info and create the secret / configmap behind the scenes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a great idea for minikube, would make it much easier to use! As far as using the addon standalone, I think that adding a config map w/ "mode" is the best solution for now.

Copy link
Member

Choose a reason for hiding this comment

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

Ok I can work on moving that into a configmap, and cleaning up the code, unless you've got something started or want to tackle.

I want to get this working @tsloughter first in this repo, then will look to incorporate into minikube which is what we had originally discussed.

kubernetes.io/minikube-addons: gcr-creds
type: Opaque
```
2. Input your application_default_credentials.json information into the secret.yaml template located [here](k8s/secret.yaml#L17):
Copy link
Member

Choose a reason for hiding this comment

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

On my mac, the application_default_credentials.jsonis empy, but my creds are incredentials`. Is this consistent?

@stevesloka
Copy link
Member

@aaron-prindle I'm going to merge this then fix one more thing, I think I see why when AWS fails then the whole loop bombs. Just need to handle the error better. I'd rather just have it try both and if one fails then it will still process the other.

Just curious about the location of the default credentials mentioned above for the docs.

@stevesloka stevesloka changed the title WIP Fixing issues with GCR Fixing issues with GCR Feb 24, 2017
@stevesloka stevesloka merged commit d65a628 into upmc-enterprises:master Feb 24, 2017
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.

3 participants