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.

Sign up for GitHub

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

Jump to bottom

Add pull secret management for DTK #110

Merged
merged 3 commits into from
Oct 14, 2021
Merged

Add pull secret management for DTK #110

merged 3 commits into from
Oct 14, 2021

Conversation

pacevedom
Copy link
Collaborator

When deploying in GCP the crane package gets into an infinite loop if traffic towards metadata server is dropped. This makes SRO unresponsive.
Changing authn discovery from crane package into a manual approach where we grab the pull secrets from a secret will avoid the use of the metadata server while still being able to authenticate and obtain the same info.

Signed-off-by: Pablo Acevedo Montserrat pacevedo@redhat.com

dagrayvid
dagrayvid reviewed Oct 8, 2021
View reviewed changes
pkg/registry/registry.go Outdated Show resolved Hide resolved
@SchSeba
Copy link

SchSeba commented Oct 11, 2021

Hi @pacevedom quick question why do you need to create an empty folder in the pod definition?

@pacevedom
Copy link
Collaborator Author

pacevedom commented Oct 11, 2021 •
edited

@SchSeba the crane package will look for existing credentials (if not configured otherwise, but this was the source of the issue in the first place) in $HOME/.docker/config.json. Since the pod is using a read-only root file system we need to create that dir so that its writable, hence the memory-backed volume.

@SchSeba
Copy link

SchSeba commented Oct 11, 2021

Oh sorry I miss that the pod file system is RO.

Thanks!
/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Oct 11, 2021

@SchSeba: changing LGTM is restricted to collaborators

In response to this:

Oh sorry I miss that the pod file system is RO.

Thanks!
/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

SchSeba
SchSeba reviewed Oct 11, 2021
View reviewed changes
Copy link

@SchSeba SchSeba left a comment

Choose a reason for hiding this comment

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

2 small comments

pkg/registry/registry.go Outdated Show resolved Hide resolved
pkg/registry/registry.go Outdated Show resolved Hide resolved
SchSeba
SchSeba reviewed Oct 13, 2021
View reviewed changes
Copy link

@SchSeba SchSeba left a comment

Choose a reason for hiding this comment

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

great just remove the duplicate variable and this PR is ready from my point of view

pkg/registry/registry.go Outdated Show resolved Hide resolved
@pacevedom
Add pull secret management
d176929 
Signed-off-by: Pablo Acevedo Montserrat <pacevedo@redhat.com>
@pacevedom
Adjust vendor dependencies
ae20992
SchSeba
SchSeba reviewed Oct 13, 2021
View reviewed changes
Copy link

@SchSeba SchSeba left a comment

Choose a reason for hiding this comment

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

lgtm

@SchSeba
Copy link

SchSeba commented Oct 13, 2021

Oh @pacevedom you should also update this file for the bundle

https://github.com/openshift-psap/special-resource-operator/blob/master/bundle/4.9/manifests/special-resource-operator.v4.9.0.clusterserviceversion.yaml

@dagrayvid
Copy link
Collaborator

/ok-to-test

@openshift-ci openshift-ci bot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Oct 13, 2021
@dagrayvid dagrayvid removed the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Oct 14, 2021
@dagrayvid
Copy link
Collaborator

/ok-to-test

@openshift-ci openshift-ci bot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Oct 14, 2021
@SchSeba
Copy link

SchSeba commented Oct 14, 2021

/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Oct 14, 2021

@SchSeba: changing LGTM is restricted to collaborators

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@SchSeba SchSeba mentioned this pull request Oct 14, 2021
Merged
@dagrayvid
Copy link
Collaborator

/approve

@openshift-ci
Copy link

openshift-ci bot commented Oct 14, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dagrayvid, pacevedom

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 14, 2021
@bthurber
Copy link
Collaborator

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2021
@openshift-merge-robot openshift-merge-robot merged commit b1a21cb into openshift-psap:master Oct 14, 2021
pacevedom added a commit to pacevedom/special-resource-operator that referenced this pull request Oct 14, 2021
@pacevedom
Add pull secret management for DTK (openshift-psap#110)
1e5f55f 
* Add pull secret management

Signed-off-by: Pablo Acevedo Montserrat <pacevedo@redhat.com>

* Adjust vendor dependencies

* Update bundle for 4.9
pacevedom added a commit to pacevedom/special-resource-operator that referenced this pull request Oct 14, 2021
@pacevedom
Add pull secret management for DTK (openshift-psap#110)
dccfd28 
* Add pull secret management

Signed-off-by: Pablo Acevedo Montserrat <pacevedo@redhat.com>

* Adjust vendor dependencies

* Update bundle for 4.9

Signed-off-by: Pablo Acevedo Montserrat <pacevedo@redhat.com>
pacevedom added a commit to pacevedom/special-resource-operator that referenced this pull request Oct 14, 2021
@pacevedom
Add pull secret management for DTK (openshift-psap#110)
c9b82d3 
* Add pull secret management

Signed-off-by: Pablo Acevedo Montserrat <pacevedo@redhat.com>

* Adjust vendor dependencies

* Update bundle for 4.9
openshift-merge-robot pushed a commit that referenced this pull request Oct 15, 2021
@pacevedom @Veritasosrb
Add pull secret management for DTK (#113)
1cff882 
* Add pull secret management for DTK (#110)

* Add pull secret management

Signed-off-by: Pablo Acevedo Montserrat <pacevedo@redhat.com>

* Adjust vendor dependencies

* Update bundle for 4.9

* Update OSMajor and OSMajorMinor when multiple kernel versions are running in cluster. Refer issue 93 for more details. (#109)

Co-authored-by: Reena Kabra <Reena.Kabra@veritas.com>

Co-authored-by: Veritas Technologies LLC <opensource@veritas.com>
Co-authored-by: Reena Kabra <Reena.Kabra@veritas.com>
ybettan pushed a commit to ybettan/special-resource-operator that referenced this pull request Nov 2, 2021
@openshift-cherrypick-robot @pacevedom @Veritasosrb
[release-4.9] Bug 2014622: Special Resource Operator(SRO) - Fail to d…
a2a3955 
…eploy simple-kmod on GCP (openshift-psap#61)

* Add pull secret management for DTK (openshift-psap#110)

* Add pull secret management

Signed-off-by: Pablo Acevedo Montserrat <pacevedo@redhat.com>

* Adjust vendor dependencies

* Update bundle for 4.9

Signed-off-by: Pablo Acevedo Montserrat <pacevedo@redhat.com>

* Update OSMajor and OSMajorMinor when multiple kernel versions are running in cluster. Refer issue 93 for more details. (openshift-psap#109)

Co-authored-by: Reena Kabra <Reena.Kabra@veritas.com>

Co-authored-by: Pablo Acevedo Montserrat <pabloacevedom@gmail.com>
Co-authored-by: Veritas Technologies LLC <opensource@veritas.com>
Co-authored-by: Reena Kabra <Reena.Kabra@veritas.com>
ybettan pushed a commit to ybettan/special-resource-operator that referenced this pull request Nov 2, 2021
@pacevedom @Veritasosrb
Bug 1996916: Special Resource Operator(SRO) - Fail to deploy simple-k…
d385d26 
…mod on GCP (openshift-psap#58)

* Add pull secret management for DTK (openshift-psap#110)

* Add pull secret management

Signed-off-by: Pablo Acevedo Montserrat <pacevedo@redhat.com>

* Adjust vendor dependencies

* Update bundle for 4.9

Signed-off-by: Pablo Acevedo Montserrat <pacevedo@redhat.com>

* Update OSMajor and OSMajorMinor when multiple kernel versions are running in cluster. Refer issue 93 for more details. (openshift-psap#109)

Co-authored-by: Reena Kabra <Reena.Kabra@veritas.com>

Co-authored-by: Veritas Technologies LLC <opensource@veritas.com>
Co-authored-by: Reena Kabra <Reena.Kabra@veritas.com>
yevgeny-shnaidman added a commit to yevgeny-shnaidman/special-resource-operator that referenced this pull request Feb 21, 2022
@yevgeny-shnaidman
MGMT-9251: Fix state machine loop regarding stateless yamls (openshif…
d02d4b6 
…t-psap#110)

This PR removes processing of the stateless yaml from the statefull
yamls state machine loop. The fixed flow is:
1) split the original chart's templates to statefull, stateless and named templates
2) first run named templates with each statefull yaml ( in order)
3) run named templates with all the stateless yaml togetter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SchSeba SchSeba SchSeba left review comments

@dagrayvid dagrayvid dagrayvid left review comments

@ybettan ybettan Awaiting requested review from ybettan

Assignees

@bthurber bthurber

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@pacevedom @SchSeba @dagrayvid @bthurber @openshift-merge-robot