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

Drop vendor registry details from container image concept #21438

Conversation

sftim
Copy link
Contributor

@sftim sftim commented Jun 2, 2020

Minimal rewrite to avoid referencing specific 3rd party vendors and their implementations.

Helps with #20232 and #21641

The amended page still needs work; that's OK, that's something we can pick up in a separate pull request.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 2, 2020
@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Jun 2, 2020
@netlify
Copy link

netlify bot commented Jun 2, 2020

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 225517a

https://deploy-preview-21438--kubernetes-io-master-staging.netlify.app

@celestehorgan
Copy link
Contributor

@sftim to be clear: when you say "The amended page still needs work;" – do you want review on grammar/language here or just for me to catch the typos and let you go on your merry way? :)

@sftim
Copy link
Contributor Author

sftim commented Jun 10, 2020

So long as these changes have no regressions, let's merge.

@@ -9,18 +9,51 @@ weight: 10

{{% capture overview %}}
Copy link
Member

@jimangel jimangel Jun 10, 2020

Choose a reason for hiding this comment

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

Do we want to replace all capture statements too?

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 we do!

Copy link
Contributor

Choose a reason for hiding this comment

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

Alas! Merge conflict, likely from #21359!

@sftim sftim force-pushed the 20200502_drop_vendor_details_from_container_image_concept branch from 19a9b6b to 0723bce Compare June 11, 2020 10:40
@jimangel
Copy link
Member

Re-triggered the build it had a strange memory error.
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 11, 2020
@jimangel
Copy link
Member

jimangel commented Jun 11, 2020

Failed again.... the logs:

9:29:19 AM: hugo --enableGitInfo --buildFuture -b https://deploy-preview-21438--kubernetes-io-master-staging.netlify.app
9:29:19 AM: Building sites …
9:29:21 AM: panic: runtime error: invalid memory address or nil pointer dereference
9:29:21 AM: [signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x109ad12]
9:29:21 AM: goroutine 84 [running]:
9:29:21 AM: github.com/gohugoio/hugo/hugolib.(*shortcodeHandler).extractShortcode(0xc0069b6c80, 0xb, 0x0, 0xc005a11090, 0x0, 0x0, 0x1a14380)
9:29:21 AM: 	/root/project/hugo/hugolib/shortcode.go:505 +0x882
9:29:21 AM: ┌─────────────────────────────┐
9:29:21 AM: │   "build.command" failed    │
9:29:21 AM: └─────────────────────────────┘
9:29:21 AM: ​
9:29:21 AM:   Error message
9:29:21 AM:   Command failed with exit code 2: make deploy-preview
9:29:21 AM: ​
9:29:21 AM:   Error location
9:29:21 AM:   In build.command from netlify.toml:
9:29:21 AM:   make deploy-preview
9:29:21 AM: ​
9:29:21 AM:   Resolved config
9:29:21 AM:   build:
9:29:21 AM:     command: make deploy-preview
9:29:21 AM:     functions: /opt/build/repo/functions
9:29:21 AM:     publish: /opt/build/repo/public

@sftim sftim force-pushed the 20200502_drop_vendor_details_from_container_image_concept branch from 0723bce to 869635d Compare June 11, 2020 23:07
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 11, 2020
Minimal rewrite to avoid referencing specific 3rd party vendors and
their implementations.
@sftim sftim force-pushed the 20200502_drop_vendor_details_from_container_image_concept branch from 869635d to 225517a Compare June 11, 2020 23:08
@tengqm
Copy link
Contributor

tengqm commented Jun 13, 2020

(re)approve
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 13, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tengqm

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 13, 2020
@k8s-ci-robot k8s-ci-robot merged commit 44acecd into kubernetes:master Jun 13, 2020
@sftim sftim deleted the 20200502_drop_vendor_details_from_container_image_concept branch September 12, 2021 10:49
@et304383
Copy link

et304383 commented Mar 4, 2022

@sftim What was the rationale for this change and where are the replacement docs that explain that K8s will automatically authenticate against AWS ECR periodically if running on EC2 with the appropriate IAM role permissions?

I just wasted a day trying to understand how K8s was authenticating in AWS when trying to replicate our cluster in OCI, and without this documentation there is NOTHING on the Internet that explains that K8s does this automatically.

@sftim
Copy link
Contributor Author

sftim commented Mar 4, 2022

This change helped with two issues: #20232 and #21641.
Kubernetes itself does not periodically authenticate against Amazon ECR. However, EC2 instances running with a particular AMI on AWS may well do that. For details on how a particular cloud provider has set up their flavor of Kubernetes, it is best to check the documentation provided by that cloud provider.

The blog article https://kubernetes.io/blog/2020/05/third-party-dual-sourced-content/ provides more content about why Kubernetes avoids documenting external / third party software.

@et304383
Copy link

et304383 commented Mar 4, 2022

This change helped with two issues: #20232 and #21641. Kubernetes itself does not periodically authenticate against Amazon ECR. However, EC2 instances running with a particular AMI on AWS may well do that. For details on how a particular cloud provider has set up their flavor of Kubernetes, it is best to check the documentation provided by that cloud provider.

Actually, it does, because we're using a non managed Kubernetes on AWS EC2 with IAM roles granting ECR access and it just works.

The K8s documentation used to state this is how it works (based on the French page). We're running on version 1.17. Are you saying this functionality is removed in later versions?

@sftim
Copy link
Contributor Author

sftim commented Mar 4, 2022

The Kubernetes documentation no longer includes documentation for third-party systems and software, other than those components necessary for Kubernetes to run.

It's possible that the kubelet does include an integration with AWS IAM and AWS ECR; if so, that's news to me. We'd be happy to document that functionality (perhaps in a different way) if it does exist as part of Kubernetes itself because that would then be built in to Kubernetes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants