Skip to content

Use different API to check assigned policy revisions#280

Merged
mtojek merged 4 commits intoelastic:masterfrom
mtojek:fix-policy-revs
Mar 9, 2021
Merged

Use different API to check assigned policy revisions#280
mtojek merged 4 commits intoelastic:masterfrom
mtojek:fix-policy-revs

Conversation

@mtojek
Copy link
Copy Markdown
Contributor

@mtojek mtojek commented Mar 9, 2021

This PR modifies test runner to use different API methods to check agents which picked up latest policy revisions (kuery has been removed).

This is a workaround for elastic/kibana#94053 .

@mtojek mtojek self-assigned this Mar 9, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Mar 9, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #280 updated

  • Start Time: 2021-03-09T14:56:35.731+0000

  • Duration: 25 min 23 sec

  • Commit: 04cc854

Test stats 🧪

Test Results
Failed 0
Passed 311
Skipped 1
Total 312

Trends 🧪

Image of Build Times

Image of Tests

var filtered []kibana.Agent
for _, agent := range allAgents {
if agent.PolicyRevision == 0 {
continue // For some reason Kibana doesn't always return a valid policy revision (eventually it will be present and valid)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not blocking this PR but it would be good to understand why this is happening and try to remove this condition in a follow up PR. @nchaulet maybe you can shed some light here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is actually causing some flakiness for Kubernetes integration. I would be for leaving it here (to unblock our delivery pipeline), but it definitely needs an explanation.

Copy link
Copy Markdown
Member

@nchaulet nchaulet Mar 9, 2021

Choose a reason for hiding this comment

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

the policy revision is set when the agent is actually running the policy it's why it could be undefined when an agent enrolled or when reassigning an agent policy

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is it by design or it just works this way? I'm concerned if it makes sense to have an agent with policy assigned, but no revision. Shouldn't it always be 1 on startup?

func (c *Client) waitUntilPolicyAssigned(p Policy) error {
totalAgents, err := c.getTotalAgentForPolicy(p)
func (c *Client) getAgent(agentID string) (*Agent, error) {
statusCode, respBody, err := c.get(fmt.Sprintf("%s/agents/%s", FleetAPI, agentID))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this API exist in older versions of Kibana? IOW, just wondering if this change is backwards compatible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it was already there, we just used a different API.

Copy link
Copy Markdown
Contributor

@ycombinator ycombinator Mar 9, 2021

Choose a reason for hiding this comment

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

I quickly tested this PR with 7.10.0 and it works. So we're good on backwards compatibility.

Copy link
Copy Markdown
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

LGTM.

It would still be good to get an answer from @nchaulet on #280. Not blocking this PR though.

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.

4 participants