Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

fix: update Container monitoring add-on with 11012019 agent image changes #2318

Merged
merged 35 commits into from
Nov 19, 2019
Merged

fix: update Container monitoring add-on with 11012019 agent image changes #2318

merged 35 commits into from
Nov 19, 2019

Conversation

ganga1980
Copy link
Contributor

@ganga1980 ganga1980 commented Nov 14, 2019

Reason for Change:

Issue Fixed:

This PR has changes related updates for the Container monitoring add-on with latest agent image version: 11012019

Requirements:

Notes:

@ganga1980
Copy link
Contributor Author

/assign @jadarsie

@devigned
Copy link
Member

/azp run pr-e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@devigned
Copy link
Member

/azp run pr-e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@devigned
Copy link
Member

/azp run pr-e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@devigned
Copy link
Member

@ganga1980 are you able to run the tests locally in the dev container? I believe this will help to avoid the test failures in the CI system.

@ganga1980
Copy link
Contributor Author

@ganga1980 are you able to run the tests locally in the dev container? I believe this will help to avoid the test failures in the CI system.
@devigned , I ran the tests locally. all the tests are getting passed. In-fact, I have ran "make ensure-generated" and also ran the test before pushing any code. Not sure why CI failing with mismatch in generated code.

@devigned
Copy link
Member

@ganga1980 I would run make reload from the tools directory to ensure you have the correct version of tools installed. see: https://github.com/Azure/aks-engine/blob/master/hack/tools/Makefile#L22

@ganga1980
Copy link
Contributor Author

@ganga1980 I would run make reload from the tools directory to ensure you have the correct version of tools installed. see: https://github.com/Azure/aks-engine/blob/master/hack/tools/Makefile#L22

@devigned, Thanks for pointing this. I was knocking my head why the generated files difference in CI not locally. I have ran "make reload", "make generate" and "make test", and tested my changes by creating cluster with addon. I think, this time would make the CI happy.

@devigned
Copy link
Member

/azp run pr-e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@codecov
Copy link

codecov bot commented Nov 15, 2019

Codecov Report

Merging #2318 into master will increase coverage by 0.01%.
The diff coverage is 75%.

@@            Coverage Diff             @@
##           master    #2318      +/-   ##
==========================================
+ Coverage   71.71%   71.73%   +0.01%     
==========================================
  Files         142      142              
  Lines       24950    24992      +42     
==========================================
+ Hits        17892    17927      +35     
- Misses       5927     5934       +7     
  Partials     1131     1131

@ganga1980
Copy link
Contributor Author

@ganga1980 I would run make reload from the tools directory to ensure you have the correct version of tools installed. see: https://github.com/Azure/aks-engine/blob/master/hack/tools/Makefile#L22

@devigned, Thanks for pointing this. I was knocking my head why the generated files difference in CI not locally. I have ran "make reload", "make generate" and "make test", and tested my changes by creating cluster with addon. I think, this time would make the CI happy.

@devigned , seems all the tests and checks are passed. when you get a chance, please help on merging this PR?

@ganga1980
Copy link
Contributor Author

Hi, @devigned , When you get a chance, can you please help on merging this PR?

@devigned
Copy link
Member

/lgtm

@acs-bot acs-bot added the lgtm label Nov 19, 2019
@acs-bot
Copy link

acs-bot commented Nov 19, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: devigned, ganga1980, rashmichandrashekar

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

@jackfrancis jackfrancis merged commit 447e9a1 into Azure:master Nov 19, 2019
@jackfrancis jackfrancis added this to the v0.45.0 milestone Dec 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants